From ac53d67cd0b89e77b9e387fae8400b22b0c158d3 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Mon, 6 May 2024 14:47:15 +0200 Subject: [PATCH 01/11] Add error message to error page --- frontend/js/src/error/ErrorBoundary.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/js/src/error/ErrorBoundary.tsx b/frontend/js/src/error/ErrorBoundary.tsx index 65ab0c76c9..0375033968 100644 --- a/frontend/js/src/error/ErrorBoundary.tsx +++ b/frontend/js/src/error/ErrorBoundary.tsx @@ -43,7 +43,8 @@ export function ErrorBoundary() { Error -

Error Occured!

+

An error occured

+

{errorMessage}

); } From 227f629af16dde14b8b2fde98f0bc4f78f26c08b Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Mon, 6 May 2024 14:48:01 +0200 Subject: [PATCH 02/11] Change base template For error pages for example which were not rendered like the rest of our react pages, because it uses a different base template --- listenbrainz/webserver/templates/art/index.html | 2 +- listenbrainz/webserver/templates/errors/base.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/listenbrainz/webserver/templates/art/index.html b/listenbrainz/webserver/templates/art/index.html index ac394d08b1..1b9d2b2c28 100644 --- a/listenbrainz/webserver/templates/art/index.html +++ b/listenbrainz/webserver/templates/art/index.html @@ -1,4 +1,4 @@ -{%- extends 'base.html' -%} +{%- extends 'index.html' -%} {%- block content -%}

ListenBrainz Cover Art Grid demo

Custom layouts

diff --git a/listenbrainz/webserver/templates/errors/base.html b/listenbrainz/webserver/templates/errors/base.html index f85cb845c0..c20739b733 100644 --- a/listenbrainz/webserver/templates/errors/base.html +++ b/listenbrainz/webserver/templates/errors/base.html @@ -1,4 +1,4 @@ -{% extends 'base.html' %} +{% extends 'index.html' %} {% block title %}{% block error_title %}{% endblock %} - ListenBrainz{% endblock %} From 076fffd691489f356ceecbf3c29dbdac8bcf9af1 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Wed, 8 May 2024 16:47:18 +0200 Subject: [PATCH 03/11] Update base templates add content block so error pages are diplayed properly if rendered from flask --- listenbrainz/webserver/templates/base.html | 7 ++----- listenbrainz/webserver/templates/index.html | 3 +++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/listenbrainz/webserver/templates/base.html b/listenbrainz/webserver/templates/base.html index a70187f600..939c79a40b 100644 --- a/listenbrainz/webserver/templates/base.html +++ b/listenbrainz/webserver/templates/base.html @@ -29,14 +29,11 @@ {% endfor %} {% endwith %} - {%- block wrapper -%} -
- +
{%- block content -%} - + {%- endblock -%}
- {%- endblock -%} {%- block footer -%}
diff --git a/listenbrainz/webserver/templates/index.html b/listenbrainz/webserver/templates/index.html index e9f1c38865..8ff8f6bbd5 100644 --- a/listenbrainz/webserver/templates/index.html +++ b/listenbrainz/webserver/templates/index.html @@ -19,6 +19,9 @@
+ {%- block content -%} + + {%- endblock -%}
From fab04b26ea76f4cf20fb1dea654c6b4066fe5c75 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Wed, 8 May 2024 16:50:34 +0200 Subject: [PATCH 04/11] Return json error if request is POST instead of returning an error HTML page, now that the front-end uses these POST requests to fetch data for the current page, expecting a JSON response --- listenbrainz/webserver/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/listenbrainz/webserver/errors.py b/listenbrainz/webserver/errors.py index f33d86ae48..9572023b94 100644 --- a/listenbrainz/webserver/errors.py +++ b/listenbrainz/webserver/errors.py @@ -180,7 +180,7 @@ def internal_server_error(error): # We specifically return json in the case that the request was within our API path original = getattr(error, "original_exception", None) - if request.path.startswith(API_PREFIX): + if request.path.startswith(API_PREFIX) or request.method == 'POST' : error = APIError("An unknown error occured.", 500) return jsonify(error.to_dict()), error.status_code else: From f2086030e83d8b4651d12d99bcf99aadac08d0aa Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Wed, 8 May 2024 16:51:31 +0200 Subject: [PATCH 05/11] pep8 --- listenbrainz/webserver/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/listenbrainz/webserver/errors.py b/listenbrainz/webserver/errors.py index 9572023b94..13f701280d 100644 --- a/listenbrainz/webserver/errors.py +++ b/listenbrainz/webserver/errors.py @@ -180,7 +180,7 @@ def internal_server_error(error): # We specifically return json in the case that the request was within our API path original = getattr(error, "original_exception", None) - if request.path.startswith(API_PREFIX) or request.method == 'POST' : + if request.path.startswith(API_PREFIX) or request.method == 'POST': error = APIError("An unknown error occured.", 500) return jsonify(error.to_dict()), error.status_code else: From cb7c0976bc79df61da773a7c99bbbc05563833ba Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Thu, 9 May 2024 12:28:53 +0200 Subject: [PATCH 06/11] Fix Fresh Releases scroll position Scroll position was not respected after navigating away and back to fresh releases. When exiting the page the scroll position was being reset to 0, so when navigating back of course it was at 0. + minor refactoring in ReleaseTimeline as there were some warnings that needed fixing. --- frontend/css/fresh-releases.less | 2 +- .../components/ReleaseCardsGrid.tsx | 2 +- .../components/ReleaseTimeline.tsx | 198 +++++++++--------- 3 files changed, 103 insertions(+), 99 deletions(-) diff --git a/frontend/css/fresh-releases.less b/frontend/css/fresh-releases.less index c6799f4b8e..ab3af15972 100644 --- a/frontend/css/fresh-releases.less +++ b/frontend/css/fresh-releases.less @@ -203,7 +203,7 @@ } } -.release-cards-grid { +#release-cards-grid { display: grid; justify-items: center; gap: 2rem; diff --git a/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx b/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx index b3d53113e1..2c55cac3ab 100644 --- a/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx +++ b/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx @@ -78,7 +78,7 @@ export default function ReleaseCardsGrid(props: ReleaseCardReleaseProps) {
{getReleaseCardGridTitle(releaseKey, order)}
-
+
{releases?.map((release) => ( , + sortDirection: string, + order: string +) { + let dataArr: Array = []; + let percentArr: Array = []; + // We want to filter out the keys that have less than 1.5% of the total releases + const minReleasesThreshold = Math.floor(releases.length * 0.015); + if (order === "release_date") { + const releasesPerDate = countBy( + releases, + (item: FreshReleaseItem) => item.release_date + ); + const filteredDates = Object.keys(releasesPerDate).filter( + (date) => releasesPerDate[date] >= minReleasesThreshold + ); + + dataArr = filteredDates.map((item) => formatReleaseDate(item)); + percentArr = filteredDates + .map((item) => (releasesPerDate[item] / releases.length) * 100) + .map((_, index, arr) => + arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) + ); + } else if (order === "artist_credit_name") { + const artistInitialsCount = countBy(releases, (item: FreshReleaseItem) => + item.artist_credit_name.charAt(0).toUpperCase() + ); + const filteredInitials = Object.keys(artistInitialsCount).filter( + (initial) => artistInitialsCount[initial] >= minReleasesThreshold + ); + + dataArr = filteredInitials.sort(); + percentArr = filteredInitials + .map((item) => (artistInitialsCount[item] / releases.length) * 100) + .map((_, index, arr) => + arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) + ); + } else if (order === "release_name") { + const releaseInitialsCount = countBy(releases, (item: FreshReleaseItem) => + item.release_name.charAt(0).toUpperCase() + ); + const filteredInitials = Object.keys(releaseInitialsCount).filter( + (initial) => releaseInitialsCount[initial] >= minReleasesThreshold + ); + + dataArr = filteredInitials.sort(); + percentArr = filteredInitials + .map((item) => (releaseInitialsCount[item] / releases.length) * 100) + .map((_, index, arr) => + arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) + ); + } else { + // conutBy gives us an asc-sorted Dict by confidence + const confidenceInitialsCount = countBy( + releases, + (item: FreshReleaseItem) => item?.confidence + ); + dataArr = Object.keys(confidenceInitialsCount); + percentArr = Object.values(confidenceInitialsCount) + .map((item) => (item / releases.length) * 100) + .map((_, index, arr) => + arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) + ); + } + + if (sortDirection === "descend") { + dataArr.reverse(); + percentArr = percentArr.reverse().map((v) => (v <= 100 ? 100 - v : 0)); + } + + /** + * We want the timeline dates or marks to start where the grid starts. + * So the 0% should always have the first date. Therefore we use unshift(0) here. + * With the same logic, we don't want the last date to be at 100% because + * that will mean we're at the bottom of the grid. + * The last date should start before 100%. That explains the pop(). + * For descending sort, the reverse computation above possibly already ensures that + * the percentArr starts with 0 and ends with a non-100% value, which is desired. + * Hence, we add a check to skip the unshift(0) and pop() operations in that case. + */ + if (percentArr[0] !== 0) { + percentArr.unshift(0); + percentArr.pop(); + } + + return zipObject(percentArr, dataArr); +} + export default function ReleaseTimeline(props: ReleaseTimelineProps) { const { releases, order, direction } = props; @@ -29,108 +118,23 @@ export default function ReleaseTimeline(props: ReleaseTimelineProps) { return scrollTo; }, []); - function createMarks(data: Array, sortDirection: string) { - let dataArr: Array = []; - let percentArr: Array = []; - // We want to filter out the keys that have less than 1.5% of the total releases - const minReleasesThreshold = Math.floor(data.length * 0.015); - if (order === "release_date") { - const releasesPerDate = countBy( - releases, - (item: FreshReleaseItem) => item.release_date - ); - const filteredDates = Object.keys(releasesPerDate).filter( - (date) => releasesPerDate[date] >= minReleasesThreshold - ); - - dataArr = filteredDates.map((item) => formatReleaseDate(item)); - percentArr = filteredDates - .map((item) => (releasesPerDate[item] / data.length) * 100) - .map((_, index, arr) => - arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) - ); - } else if (order === "artist_credit_name") { - const artistInitialsCount = countBy(data, (item: FreshReleaseItem) => - item.artist_credit_name.charAt(0).toUpperCase() - ); - const filteredInitials = Object.keys(artistInitialsCount).filter( - (initial) => artistInitialsCount[initial] >= minReleasesThreshold - ); - - dataArr = filteredInitials.sort(); - percentArr = filteredInitials - .map((item) => (artistInitialsCount[item] / data.length) * 100) - .map((_, index, arr) => - arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) - ); - } else if (order === "release_name") { - const releaseInitialsCount = countBy(data, (item: FreshReleaseItem) => - item.release_name.charAt(0).toUpperCase() - ); - const filteredInitials = Object.keys(releaseInitialsCount).filter( - (initial) => releaseInitialsCount[initial] >= minReleasesThreshold - ); - - dataArr = filteredInitials.sort(); - percentArr = filteredInitials - .map((item) => (releaseInitialsCount[item] / data.length) * 100) - .map((_, index, arr) => - arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) - ); - } else { - // conutBy gives us an asc-sorted Dict by confidence - const confidenceInitialsCount = countBy( - data, - (item: FreshReleaseItem) => item?.confidence - ); - dataArr = Object.keys(confidenceInitialsCount); - percentArr = Object.values(confidenceInitialsCount) - .map((item) => (item / data.length) * 100) - .map((_, index, arr) => - arr.slice(0, index + 1).reduce((prev, curr) => prev + curr) - ); - } - - if (sortDirection === "descend") { - dataArr.reverse(); - percentArr = percentArr.reverse().map((v) => (v <= 100 ? 100 - v : 0)); - } - - /** - * We want the timeline dates or marks to start where the grid starts. - * So the 0% should always have the first date. Therefore we use unshift(0) here. - * With the same logic, we don't want the last date to be at 100% because - * that will mean we're at the bottom of the grid. - * The last date should start before 100%. That explains the pop(). - * For descending sort, the reverse computation above possibly already ensures that - * the percentArr starts with 0 and ends with a non-100% value, which is desired. - * Hence, we add a check to skip the unshift(0) and pop() operations in that case. - */ - if (percentArr[0] !== 0) { - percentArr.unshift(0); - percentArr.pop(); - } - - return zipObject(percentArr, dataArr); - } - - const handleScroll = React.useCallback( - debounce(() => { - // TODO change to relative position of #release-cards-grid instead of window - const scrollPos = - (window.scrollY / document.documentElement.scrollHeight) * 100; - setCurrentValue(scrollPos); - }, 300), - [] - ); - React.useEffect(() => { - setMarks(createMarks(releases, direction)); - }, [releases, direction]); + setMarks(createMarks(releases, direction, order)); + }, [releases, direction, order]); React.useEffect(() => { + const handleScroll = debounce(() => { + const container = document.getElementById("#release-cards-grid"); + if (!container) { + return; + } + const scrollPos = (window.scrollY / container.scrollHeight) * 100; + setCurrentValue(scrollPos); + }, 500); + window.addEventListener("scroll", handleScroll); return () => { + handleScroll.cancel(); window.removeEventListener("scroll", handleScroll); }; }, []); From ef15094bf73d4b8de7ea48e22a16575b3809b729 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Thu, 9 May 2024 13:45:31 +0200 Subject: [PATCH 07/11] FreshReleases get correct container element + revert previous changes from class to id , got confused by "release-card-grids" vs. "release-cards-grid" --- frontend/css/fresh-releases.less | 2 +- .../explore/fresh-releases/components/ReleaseCardsGrid.tsx | 2 +- .../explore/fresh-releases/components/ReleaseTimeline.tsx | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/css/fresh-releases.less b/frontend/css/fresh-releases.less index ab3af15972..c6799f4b8e 100644 --- a/frontend/css/fresh-releases.less +++ b/frontend/css/fresh-releases.less @@ -203,7 +203,7 @@ } } -#release-cards-grid { +.release-cards-grid { display: grid; justify-items: center; gap: 2rem; diff --git a/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx b/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx index 2c55cac3ab..b3d53113e1 100644 --- a/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx +++ b/frontend/js/src/explore/fresh-releases/components/ReleaseCardsGrid.tsx @@ -78,7 +78,7 @@ export default function ReleaseCardsGrid(props: ReleaseCardReleaseProps) {
{getReleaseCardGridTitle(releaseKey, order)}
-
+
{releases?.map((release) => ( { const handleScroll = debounce(() => { - const container = document.getElementById("#release-cards-grid"); + const container = document.getElementById("release-card-grids"); if (!container) { return; } - const scrollPos = (window.scrollY / container.scrollHeight) * 100; + const scrollPos = + ((window.scrollY - container.offsetTop) / container.scrollHeight) * 100; setCurrentValue(scrollPos); }, 500); From 3b69c45ec5fca69c3567af3eafaf7289c8315921 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Thu, 9 May 2024 16:32:25 +0200 Subject: [PATCH 08/11] Refactor error handling response Set accept header to get json response instead of HTML page. Also refactor ISE handler which was duplicating some logic in handle_error. --- frontend/js/src/utils/Loader.ts | 4 ++-- listenbrainz/webserver/errors.py | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/frontend/js/src/utils/Loader.ts b/frontend/js/src/utils/Loader.ts index 7428129b0f..4603e8e35d 100644 --- a/frontend/js/src/utils/Loader.ts +++ b/frontend/js/src/utils/Loader.ts @@ -8,7 +8,7 @@ const RouteLoader = async ({ request }: { request: Request }) => { const response = await fetch(request.url, { method: "POST", headers: { - "Content-Type": "application/json", + Accept: "application/json", }, }); const data = await response.json(); @@ -24,7 +24,7 @@ const RouteLoaderURL = async (url: string) => { const response = await fetch(url, { method: "POST", headers: { - "Content-Type": "application/json", + Accept: "application/json", }, }); const data = await response.json(); diff --git a/listenbrainz/webserver/errors.py b/listenbrainz/webserver/errors.py index 13f701280d..de4ec799c7 100644 --- a/listenbrainz/webserver/errors.py +++ b/listenbrainz/webserver/errors.py @@ -141,7 +141,7 @@ def handle_error(error, code): A Response which will be a json error if request was made to the LB api and an html page otherwise """ - if current_app.config.get('IS_API_COMPAT_APP') or request.path.startswith(API_PREFIX): + if current_app.config.get('IS_API_COMPAT_APP') or request.path.startswith(API_PREFIX) or request.accept_mimetypes.accept_json: response = jsonify({'code': code, 'error': error.description}) response.headers["Access-Control-Allow-Origin"] = "*" return response, code @@ -180,11 +180,7 @@ def internal_server_error(error): # We specifically return json in the case that the request was within our API path original = getattr(error, "original_exception", None) - if request.path.startswith(API_PREFIX) or request.method == 'POST': - error = APIError("An unknown error occured.", 500) - return jsonify(error.to_dict()), error.status_code - else: - return handle_error(original or error, 500) + return handle_error(original or error, 500) @app.errorhandler(502) def bad_gateway(error): From 1c9b517206eccb4cbe21c153a70840ffdcf2ec0f Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Fri, 10 May 2024 09:37:11 +0530 Subject: [PATCH 09/11] Fix handle_error Some exceptions like DatabaseError do not have a description attribute, therefore check for the presence of description attr before accessing it and default to a generic fallback message if not present. --- listenbrainz/webserver/errors.py | 14 ++++++++++---- listenbrainz/webserver/test/test_api_errors.py | 5 ++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/listenbrainz/webserver/errors.py b/listenbrainz/webserver/errors.py index de4ec799c7..c125bd2ed5 100644 --- a/listenbrainz/webserver/errors.py +++ b/listenbrainz/webserver/errors.py @@ -141,8 +141,14 @@ def handle_error(error, code): A Response which will be a json error if request was made to the LB api and an html page otherwise """ - if current_app.config.get('IS_API_COMPAT_APP') or request.path.startswith(API_PREFIX) or request.accept_mimetypes.accept_json: - response = jsonify({'code': code, 'error': error.description}) + if current_app.config.get('IS_API_COMPAT_APP') or \ + request.path.startswith(API_PREFIX) or \ + request.accept_mimetypes.accept_json: + if hasattr(error, "description"): + description = error.description + else: + description = "An unknown error occured." + response = jsonify({"code": code, "error": description}) response.headers["Access-Control-Allow-Origin"] = "*" return response, code return error_wrapper('errors/{code}.html'.format(code=code), error, code) @@ -208,6 +214,7 @@ def handle_api_compat_error(error): def handle_playlist_api_xml_error(error): return error.render_error() + class InvalidAPIUsage(Exception): """ General error class for the API_compat to render errors in multiple formats """ @@ -241,8 +248,6 @@ def to_xml(self): return '\n' + yattag.indent(doc.getvalue()) - - class PlaylistAPIXMLError(Exception): """ Custom error class for Playlist API to render errors in XML format. @@ -265,6 +270,7 @@ def to_xml(self): text(self.message) return '\n' + yattag.indent(doc.getvalue()) + class ListenValidationError(Exception): """ Error class for raising when the submitted payload does not pass validation. Only use for code paths common to LB API, API compat & API compat deprecated. diff --git a/listenbrainz/webserver/test/test_api_errors.py b/listenbrainz/webserver/test/test_api_errors.py index 6b6aa40955..f053d55fa2 100644 --- a/listenbrainz/webserver/test/test_api_errors.py +++ b/listenbrainz/webserver/test/test_api_errors.py @@ -1,3 +1,5 @@ +from psycopg2 import DatabaseError + from listenbrainz.db.testing import DatabaseTestCase from listenbrainz.webserver.testing import ServerTestCase @@ -10,10 +12,11 @@ class APIErrorTestCase(DatabaseTestCase, ServerTestCase): def setUp(self): ServerTestCase.setUp(self) DatabaseTestCase.setUp(self) + self.app.config["TESTING"] = False @mock.patch('listenbrainz.webserver.views.stats_api.db_user.get_by_mb_id') def test_api_error_unexpected_error_returns_json(self, mock_db_get): - mock_db_get.side_effect = InternalServerError("Some bad thing happened") + mock_db_get.side_effect = DatabaseError() r = self.client.get('/1/stats/user/iliekcomputers/artists') self.assert500(r) self.assertEqual(r.json['error'], 'An unknown error occured.') From ced8901d38ce01d24407d00fffa667a37d25501c Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Fri, 10 May 2024 12:09:48 +0200 Subject: [PATCH 10/11] Handle UserReports errors Handle errors gracefully instead of throwing an error if we get a rate limit issue, which replaces the entire page with the ErrorBoundary --- frontend/js/src/user/stats/UserReports.tsx | 61 ++++++++----------- .../user/stats/components/UserArtistMap.tsx | 15 ++--- .../stats/components/UserDailyActivity.tsx | 15 ++--- .../components/UserListeningActivity.tsx | 15 ++--- .../user/stats/components/UserTopEntity.tsx | 15 ++--- frontend/js/src/utils/APIService.ts | 16 +++-- 6 files changed, 56 insertions(+), 81 deletions(-) diff --git a/frontend/js/src/user/stats/UserReports.tsx b/frontend/js/src/user/stats/UserReports.tsx index 1afff528d5..c1930c924f 100644 --- a/frontend/js/src/user/stats/UserReports.tsx +++ b/frontend/js/src/user/stats/UserReports.tsx @@ -10,7 +10,6 @@ import { useLoaderData, useNavigate } from "react-router-dom"; import type { NavigateFunction } from "react-router-dom"; import { Helmet } from "react-helmet"; -import ErrorBoundary from "../../utils/ErrorBoundary"; import Pill from "../../components/Pill"; import UserListeningActivity from "./components/UserListeningActivity"; import UserTopEntity from "./components/UserTopEntity"; @@ -170,58 +169,46 @@ export default class UserReports extends React.Component<
- - - +
- - - +
- - - +
- - - +
{user && (
- - - +
)}
- - - +
); diff --git a/frontend/js/src/user/stats/components/UserArtistMap.tsx b/frontend/js/src/user/stats/components/UserArtistMap.tsx index 5b783cb6b8..921a6c7951 100644 --- a/frontend/js/src/user/stats/components/UserArtistMap.tsx +++ b/frontend/js/src/user/stats/components/UserArtistMap.tsx @@ -98,16 +98,11 @@ export default class UserArtistMap extends React.Component< try { return await this.APIService.getUserArtistMap(user?.name, range); } catch (error) { - if (error.response && error.response.status === 204) { - this.setState({ - loading: false, - hasError: true, - errorMessage: - "There are no statistics available for this user for this period", - }); - } else { - throw error; - } + this.setState({ + loading: false, + hasError: true, + errorMessage: error.message, + }); } return {} as UserArtistMapResponse; }; diff --git a/frontend/js/src/user/stats/components/UserDailyActivity.tsx b/frontend/js/src/user/stats/components/UserDailyActivity.tsx index 7a6141576f..1b831f6f22 100644 --- a/frontend/js/src/user/stats/components/UserDailyActivity.tsx +++ b/frontend/js/src/user/stats/components/UserDailyActivity.tsx @@ -66,16 +66,11 @@ export default class UserDailyActivity extends React.Component< const data = await this.APIService.getUserDailyActivity(user.name, range); return data; } catch (error) { - if (error.response && error.response.status === 204) { - this.setState({ - loading: false, - hasError: true, - errorMessage: - "There are no statistics available for this user for this period", - }); - } else { - throw error; - } + this.setState({ + loading: false, + hasError: true, + errorMessage: error.message, + }); } return {} as UserDailyActivityResponse; }; diff --git a/frontend/js/src/user/stats/components/UserListeningActivity.tsx b/frontend/js/src/user/stats/components/UserListeningActivity.tsx index 2eb7c11abe..c7ab1f6471 100644 --- a/frontend/js/src/user/stats/components/UserListeningActivity.tsx +++ b/frontend/js/src/user/stats/components/UserListeningActivity.tsx @@ -133,16 +133,11 @@ export default class UserListeningActivity extends React.Component< try { return await this.APIService.getUserListeningActivity(user?.name, range); } catch (error) { - if (error.response && error.response.status === 204) { - this.setState({ - loading: false, - hasError: true, - errorMessage: - "There are no statistics available for this user for this period", - }); - } else { - throw error; - } + this.setState({ + loading: false, + hasError: true, + errorMessage: error.message, + }); } return {} as UserListeningActivityResponse; }; diff --git a/frontend/js/src/user/stats/components/UserTopEntity.tsx b/frontend/js/src/user/stats/components/UserTopEntity.tsx index a9d26229d9..8bc0775b24 100644 --- a/frontend/js/src/user/stats/components/UserTopEntity.tsx +++ b/frontend/js/src/user/stats/components/UserTopEntity.tsx @@ -86,16 +86,11 @@ export default class UserTopEntity extends React.Component< 10 ); } catch (error) { - if (error.response && error.response.status === 204) { - this.setState({ - loading: false, - hasError: true, - errorMessage: - "There are no statistics available for this user for this period", - }); - } else { - throw error; - } + this.setState({ + loading: false, + hasError: true, + errorMessage: error.message, + }); } return {} as UserEntityResponse; }; diff --git a/frontend/js/src/utils/APIService.ts b/frontend/js/src/utils/APIService.ts index 45c9af9dfd..7440faa252 100644 --- a/frontend/js/src/utils/APIService.ts +++ b/frontend/js/src/utils/APIService.ts @@ -429,7 +429,9 @@ export default class APIService { await this.checkStatus(response); // if response code is 204, then statistics havent been calculated, send empty object if (response.status === 204) { - const error = new APIError(`HTTP Error ${response.statusText}`); + const error = new APIError( + "There are no statistics available for this user for this period" + ); error.status = response.statusText; error.response = response; throw error; @@ -450,7 +452,9 @@ export default class APIService { const response = await fetch(`${url}?range=${range}`); await this.checkStatus(response); if (response.status === 204) { - const error = new APIError(`HTTP Error ${response.statusText}`); + const error = new APIError( + "There are no statistics available for this user for this period" + ); error.status = response.statusText; error.response = response; throw error; @@ -466,7 +470,9 @@ export default class APIService { const response = await fetch(url); await this.checkStatus(response); if (response.status === 204) { - const error = new APIError(`HTTP Error ${response.statusText}`); + const error = new APIError( + "There are no statistics available for this user for this period" + ); error.status = response.statusText; error.response = response; throw error; @@ -489,7 +495,9 @@ export default class APIService { const response = await fetch(url); await this.checkStatus(response); if (response.status === 204) { - const error = new APIError(`HTTP Error ${response.statusText}`); + const error = new APIError( + "There are no statistics available for this user for this period" + ); error.status = response.statusText; error.response = response; throw error; From cf0e31854ca69e47aba09e6bcdcdcf64b7a88abc Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Fri, 10 May 2024 12:31:00 +0200 Subject: [PATCH 11/11] Fix UserReports tests --- .../tests/user/stats/UserArtistMap.test.tsx | 20 +++--------- .../user/stats/UserDailyActivity.test.tsx | 22 +++---------- .../user/stats/UserListeningActivity.test.tsx | 31 ++++++------------- .../tests/user/stats/UserTopEntity.test.tsx | 19 +----------- frontend/js/tests/utils/APIService.test.ts | 20 ++++++------ 5 files changed, 29 insertions(+), 83 deletions(-) diff --git a/frontend/js/tests/user/stats/UserArtistMap.test.tsx b/frontend/js/tests/user/stats/UserArtistMap.test.tsx index b0828b1ed2..8f83e99171 100644 --- a/frontend/js/tests/user/stats/UserArtistMap.test.tsx +++ b/frontend/js/tests/user/stats/UserArtistMap.test.tsx @@ -55,7 +55,9 @@ describe.each([ }); await waitForComponentToPaint(wrapper); - expect(wrapper.getDOMNode()).toHaveTextContent("Invalid range: invalid_range"); + expect(wrapper.getDOMNode()).toHaveTextContent( + "Invalid range: invalid_range" + ); expect(wrapper.find(CustomChoropleth)).toHaveLength(0); }); }); @@ -123,23 +125,9 @@ describe.each([ expect(wrapper.state()).toMatchObject({ loading: false, hasError: true, - errorMessage: "There are no statistics available for this user for this period", + errorMessage: "NO CONTENT", }); }); - - it("throws error", async () => { - const wrapper = shallow(); - const instance = wrapper.instance(); - - const spy = jest.spyOn(instance.APIService, "getUserArtistMap"); - const notFoundError = new APIError("NOT FOUND"); - notFoundError.response = { - status: 404, - } as Response; - spy.mockImplementation(() => Promise.reject(notFoundError)); - - await expect(instance.getData()).rejects.toThrow("NOT FOUND"); - }); }); describe("processData", () => { diff --git a/frontend/js/tests/user/stats/UserDailyActivity.test.tsx b/frontend/js/tests/user/stats/UserDailyActivity.test.tsx index 8c37f51c1e..77cabde4df 100644 --- a/frontend/js/tests/user/stats/UserDailyActivity.test.tsx +++ b/frontend/js/tests/user/stats/UserDailyActivity.test.tsx @@ -47,7 +47,9 @@ describe("UserDailyActivity", () => { }); await waitForComponentToPaint(wrapper); - expect(wrapper.getDOMNode()).toHaveTextContent("Invalid range: invalid_range"); + expect(wrapper.getDOMNode()).toHaveTextContent( + "Invalid range: invalid_range" + ); expect(wrapper.find(Heatmap)).toHaveLength(0); }); @@ -121,25 +123,9 @@ describe("UserDailyActivity", () => { expect(wrapper.state()).toMatchObject({ loading: false, hasError: true, - errorMessage: "There are no statistics available for this user for this period", + errorMessage: "NO CONTENT", }); }); - - it("throws error", async () => { - const wrapper = mount( - - ); - const instance = wrapper.instance(); - - const spy = jest.spyOn(instance.APIService, "getUserDailyActivity"); - const notFoundError = new APIError("NOT FOUND"); - notFoundError.response = { - status: 404, - } as Response; - spy.mockImplementation(() => Promise.reject(notFoundError)); - - await expect(instance.getData()).rejects.toThrow("NOT FOUND"); - }); }); describe("processData", () => { diff --git a/frontend/js/tests/user/stats/UserListeningActivity.test.tsx b/frontend/js/tests/user/stats/UserListeningActivity.test.tsx index c2494c767d..b5c22dfd88 100644 --- a/frontend/js/tests/user/stats/UserListeningActivity.test.tsx +++ b/frontend/js/tests/user/stats/UserListeningActivity.test.tsx @@ -2,6 +2,8 @@ import * as React from "react"; import { mount, ReactWrapper, shallow, ShallowWrapper } from "enzyme"; import { act } from "react-dom/test-utils"; +import { ResponsiveBar } from "@nivo/bar"; +import { Context as ResponsiveContext } from "react-responsive"; import UserListeningActivity, { UserListeningActivityProps, UserListeningActivityState, @@ -16,8 +18,6 @@ import * as userListeningActivityProcessedDataMonth from "../../__mocks__/userLi import * as userListeningActivityProcessedDataYear from "../../__mocks__/userListeningActivityProcessDataYear.json"; import * as userListeningActivityProcessedDataAllTime from "../../__mocks__/userListeningActivityProcessDataAllTime.json"; import { waitForComponentToPaint } from "../../test-utils"; -import { ResponsiveBar } from "@nivo/bar"; -import { Context as ResponsiveContext } from 'react-responsive' const userProps: UserListeningActivityProps = { user: { @@ -124,13 +124,18 @@ describe.each([ it("renders corectly when range is invalid", async () => { const wrapper = mount( - + ); await waitForComponentToPaint(wrapper); expect(wrapper.state().hasError).toBeTruthy(); expect(wrapper.find(ResponsiveBar)).toHaveLength(0); - expect(wrapper.getDOMNode()).toHaveTextContent("Invalid range: invalid_range"); + expect(wrapper.getDOMNode()).toHaveTextContent( + "Invalid range: invalid_range" + ); }); }); @@ -205,25 +210,9 @@ describe.each([ expect(wrapper.state()).toMatchObject({ loading: false, hasError: true, - errorMessage: "There are no statistics available for this user for this period", + errorMessage: "NO CONTENT", }); }); - - it("throws error", async () => { - const wrapper = shallow( - - ); - const instance = wrapper.instance(); - - const spy = jest.spyOn(instance.APIService, "getUserListeningActivity"); - const notFoundError = new APIError("NOT FOUND"); - notFoundError.response = { - status: 404, - } as Response; - spy.mockImplementation(() => Promise.reject(notFoundError)); - - await expect(instance.getData()).rejects.toThrow("NOT FOUND"); - }); }); describe("getNumberOfDaysInMonth", () => { diff --git a/frontend/js/tests/user/stats/UserTopEntity.test.tsx b/frontend/js/tests/user/stats/UserTopEntity.test.tsx index e7c070f5bc..12283bd49e 100644 --- a/frontend/js/tests/user/stats/UserTopEntity.test.tsx +++ b/frontend/js/tests/user/stats/UserTopEntity.test.tsx @@ -265,26 +265,9 @@ describe.each([ expect(childElement.state()).toMatchObject({ loading: false, hasError: true, - errorMessage: - "There are no statistics available for this user for this period", + errorMessage: "NO CONTENT", }); wrapper.unmount(); }); - - it("throws error", async () => { - const wrapper = shallow(getComponent(props)); - const childElement = shallow(wrapper.find(UserTopEntity).get(0)); - const instance = childElement.instance() as UserTopEntity; - - const spy = jest.spyOn(instance.APIService, "getUserEntity"); - const notFoundError = new APIError("NOT FOUND"); - notFoundError.response = { - status: 404, - } as Response; - spy.mockImplementation(() => Promise.reject(notFoundError)); - - await expect(instance.getData()).rejects.toThrow("NOT FOUND"); - wrapper.unmount(); - }); }); }); diff --git a/frontend/js/tests/utils/APIService.test.ts b/frontend/js/tests/utils/APIService.test.ts index eb0799653b..2f6653465e 100644 --- a/frontend/js/tests/utils/APIService.test.ts +++ b/frontend/js/tests/utils/APIService.test.ts @@ -15,7 +15,7 @@ describe("submitListens", () => { status: 200, }); }); - jest.useFakeTimers({advanceTimers: true}); + jest.useFakeTimers({ advanceTimers: true }); }); it("calls fetch with correct parameters", async () => { @@ -73,7 +73,7 @@ describe("submitListens", () => { }, }, ]); - + await jest.advanceTimersByTimeAsync(10000); expect(spy).toHaveBeenCalledTimes(2); @@ -321,12 +321,12 @@ describe("getUserEntity", () => { return Promise.resolve({ ok: true, status: 204, - statusText: "NO CONTENT", + statusText: "Whatever error", }); }); await expect(apiService.getUserEntity("foobar", "artist")).rejects.toThrow( - Error("HTTP Error NO CONTENT") + Error("There are no statistics available for this user for this period") ); }); @@ -376,12 +376,12 @@ describe("getUserListeningActivity", () => { return Promise.resolve({ ok: true, status: 204, - statusText: "NO CONTENT", + statusText: "Whatever error", }); }); await expect(apiService.getUserListeningActivity("foobar")).rejects.toThrow( - Error("HTTP Error NO CONTENT") + Error("There are no statistics available for this user for this period") ); }); @@ -424,12 +424,12 @@ describe("getUserDailyActivity", () => { return Promise.resolve({ ok: true, status: 204, - statusText: "NO CONTENT", + statusText: "Whatever error", }); }); await expect(apiService.getUserDailyActivity("foobar")).rejects.toThrow( - Error("HTTP Error NO CONTENT") + Error("There are no statistics available for this user for this period") ); }); @@ -479,12 +479,12 @@ describe("getUserArtistMap", () => { return Promise.resolve({ ok: true, status: 204, - statusText: "NO CONTENT", + statusText: "Whatever error", }); }); await expect(apiService.getUserArtistMap("foobar")).rejects.toThrow( - Error("HTTP Error NO CONTENT") + Error("There are no statistics available for this user for this period") ); });