diff --git a/backend/caviardeul/management/commands/check_next_article.py b/backend/caviardeul/management/commands/check_next_article.py index 934efc4..cc1d4e1 100644 --- a/backend/caviardeul/management/commands/check_next_article.py +++ b/backend/caviardeul/management/commands/check_next_article.py @@ -16,9 +16,6 @@ def handle(self, *args, **options): raise CommandError("No daily article left") try: - title, text = get_article_html_from_wikipedia(next_article.page_id) + get_article_html_from_wikipedia(next_article.page_id) except ArticleFetchError: raise CommandError("Error when retrieving daily article") - - if "redirectMsg" in text: - raise CommandError("Next daily article has a redirect") diff --git a/backend/caviardeul/serializers/error.py b/backend/caviardeul/serializers/error.py new file mode 100644 index 0000000..1ccd0fe --- /dev/null +++ b/backend/caviardeul/serializers/error.py @@ -0,0 +1,5 @@ +from ninja import Schema + + +class ErrorSchema(Schema): + detail: str diff --git a/backend/caviardeul/services/articles.py b/backend/caviardeul/services/articles.py index 91555a5..eb4cb41 100644 --- a/backend/caviardeul/services/articles.py +++ b/backend/caviardeul/services/articles.py @@ -53,14 +53,19 @@ def get_article_html_from_wikipedia(page_id: str) -> tuple[str, str]: }, ) if response.status_code != 200: - raise ArticleFetchError + raise ArticleFetchError(f"Unexected response from API: {response.status_code}") data = response.json() if "error" in data: - raise ArticleFetchError + raise ArticleFetchError("Error received in API response") data = data["parse"] - return data["title"], data["text"] + html_content = data["text"] + + if 'class="redirectMsg"' in html_content: + raise ArticleFetchError("Redirection received in article payload") + + return data["title"], html_content def _prepare_article_content_from_html(page_title: str, html_content: str) -> str: diff --git a/backend/caviardeul/services/daily_article.py b/backend/caviardeul/services/daily_article.py new file mode 100644 index 0000000..7fc19b1 --- /dev/null +++ b/backend/caviardeul/services/daily_article.py @@ -0,0 +1,10 @@ +from django.utils import timezone + +from caviardeul.models import DailyArticle + + +def get_current_daily_article_id() -> int | None: + article = ( + DailyArticle.objects.filter(date__lt=timezone.now()).order_by("-date").first() + ) + return article.id if article else None diff --git a/backend/caviardeul/tests/conftest.py b/backend/caviardeul/tests/conftest.py index 3d42eb4..1cd0b95 100644 --- a/backend/caviardeul/tests/conftest.py +++ b/backend/caviardeul/tests/conftest.py @@ -41,6 +41,36 @@ def inner( return inner +@pytest.fixture() +def mock_wiki_api_redirect(httpx_mock): + def inner(page_id: str, title: str): + httpx_mock.add_response( + url=httpx.URL( + "https://fr.wikipedia.org/w/api.php", + params={ + "action": "parse", + "format": "json", + "prop": "text", + "formatversion": 2, + "origin": "*", + "page": page_id, + }, + ), + json={ + "parse": { + "title": title, + "text": ( + '

Rediriger vers :

' + '' + "
" + ), + } + }, + ) + + return inner + + @pytest.fixture() def mock_wiki_api_error(httpx_mock): def inner(page_id: str): diff --git a/backend/caviardeul/tests/management/commands/test_check_next_article.py b/backend/caviardeul/tests/management/commands/test_check_next_article.py index e5dc85c..ea76a08 100644 --- a/backend/caviardeul/tests/management/commands/test_check_next_article.py +++ b/backend/caviardeul/tests/management/commands/test_check_next_article.py @@ -50,7 +50,7 @@ def test_check_next_article_has_redirect(mock_wiki_api): '', ) - with pytest.raises(CommandError, match="Next daily article has a redirect"): + with pytest.raises(CommandError, match="Error when retrieving daily article"): call_command("check_next_article") diff --git a/backend/caviardeul/tests/views/test_daily_article.py b/backend/caviardeul/tests/views/test_daily_article.py index d9aa34d..5dd5a7c 100644 --- a/backend/caviardeul/tests/views/test_daily_article.py +++ b/backend/caviardeul/tests/views/test_daily_article.py @@ -1,3 +1,4 @@ +import logging from collections import defaultdict from datetime import datetime from typing import Literal @@ -110,11 +111,25 @@ def test_get_current_article( "nbWinners": article.nb_winners, } + def test_error_with_article(self, client, caplog, mock_wiki_api_redirect): + article = DailyArticleFactory(trait_current=True) + mock_wiki_api_redirect(article.page_id, article.page_name) + + with caplog.at_level(logging.ERROR): + res = client.get("/articles/current") + assert "Redirection received in article payload" in caplog.text + + assert res.status_code == 500, res.content + data = res.json() + assert data["detail"] + def test_no_article_available(self, client): _ = DailyArticleFactory(trait_future=True) res = client.get("/articles/current") assert res.status_code == 404, res.content + data = res.json() + assert data["detail"] class TestGetAchivedArticle: diff --git a/backend/caviardeul/views/daily_article.py b/backend/caviardeul/views/daily_article.py index 47e1bd9..6281c6b 100644 --- a/backend/caviardeul/views/daily_article.py +++ b/backend/caviardeul/views/daily_article.py @@ -1,12 +1,13 @@ from datetime import timedelta from django.contrib.auth.models import AnonymousUser -from django.db.models import Avg, Count, FilteredRelation, Q -from django.http import Http404, HttpRequest +from django.db.models import Avg, Count, FilteredRelation, Q, QuerySet +from django.http import HttpRequest from django.utils import timezone from ninja import Query from ninja.pagination import paginate +from caviardeul.exceptions import ArticleFetchError from caviardeul.models import DailyArticle, User from caviardeul.serializers.daily_article import ( DailyArticleListFilter, @@ -15,12 +16,29 @@ DailyArticleSchema, DailyArticlesStatsSchema, ) +from caviardeul.serializers.error import ErrorSchema from caviardeul.services.articles import get_article_content from caviardeul.services.authentication import OptionalAPIAuthentication +from caviardeul.services.daily_article import get_current_daily_article_id +from caviardeul.services.logging import logger from .api import api +@api.get( + "/articles/stats", + auth=OptionalAPIAuthentication(), + response=DailyArticlesStatsSchema, +) +def get_daily_article_stats(request: HttpRequest): + return _get_queryset(request.auth).aggregate( + total=Count("id"), + total_finished=Count("id", filter=Q(user_score__isnull=False)), + average_nb_attempts=Avg("user_score__nb_attempts"), + average_nb_correct=Avg("user_score__nb_correct"), + ) + + def _get_queryset(user: User | AnonymousUser): queryset = DailyArticle.objects.filter(date__lte=timezone.now()) if not user.is_authenticated: @@ -35,44 +53,41 @@ def _get_queryset(user: User | AnonymousUser): @api.get( - "/articles/current", auth=OptionalAPIAuthentication(), response=DailyArticleSchema -) -def get_current_article(request: HttpRequest): - try: - article = _get_queryset(request.auth).order_by("-date")[:1].get() - except DailyArticle.DoesNotExist: - raise Http404("L'article n'a pas été trouvé") - - article.content = get_article_content(article) - return article - - -@api.get( - "/articles/stats", + "/articles/current", auth=OptionalAPIAuthentication(), - response=DailyArticlesStatsSchema, + response={200: DailyArticleSchema, 404: ErrorSchema, 500: ErrorSchema}, ) -def get_daily_article_stats(request: HttpRequest): - return _get_queryset(request.auth).aggregate( - total=Count("id"), - total_finished=Count("id", filter=Q(user_score__isnull=False)), - average_nb_attempts=Avg("user_score__nb_attempts"), - average_nb_correct=Avg("user_score__nb_correct"), +def get_current_article(request: HttpRequest): + article_id = get_current_daily_article_id() + return _get_daily_article_response( + _get_queryset(request.auth).filter(id=article_id) ) @api.get( "/articles/{article_id}", auth=OptionalAPIAuthentication(), - response=DailyArticleSchema, + response={200: DailyArticleSchema, 404: ErrorSchema, 500: ErrorSchema}, ) def get_archived_article(request: HttpRequest, article_id: int): + return _get_daily_article_response( + _get_queryset(request.auth).filter(id=article_id) + ) + + +def _get_daily_article_response(queryset: QuerySet[DailyArticle]): try: - article = _get_queryset(request.auth).get(id=article_id) + article = queryset.get() except DailyArticle.DoesNotExist: - raise Http404("L'article n'a pas été trouvé") + return 404, {"detail": "L'article n'a pas été trouvé"} - article.content = get_article_content(article) + try: + article.content = get_article_content(article) + except ArticleFetchError: + logger.exception( + "Error encountered with daily article", extra={"article_id": article.id} + ) + return 500, {"detail": "Un problème a été rencontré avec cet article"} return article diff --git a/frontend/components/utils/error.tsx b/frontend/components/utils/error.tsx new file mode 100644 index 0000000..fc670f7 --- /dev/null +++ b/frontend/components/utils/error.tsx @@ -0,0 +1,15 @@ +import React from "react"; +import Error from "next/error"; + + +const CustomError: React.FC<{ statusCode?: number, text?: string }> = ({statusCode, text}) => { + return ( +
+
+ +
+
+ ) +} + +export default CustomError diff --git a/frontend/lib/article.ts b/frontend/lib/article.ts index 9c349e9..6cf1611 100644 --- a/frontend/lib/article.ts +++ b/frontend/lib/article.ts @@ -1,5 +1,6 @@ import { ArticleId, EncodedArticle } from "@caviardeul/types"; import { API_URL } from "@caviardeul/utils/config"; +import {APIError} from "@caviardeul/lib/queries"; export const getEncodedArticle = async ( articleId?: ArticleId, @@ -23,7 +24,7 @@ export const getEncodedArticle = async ( const data = await res.json(); if (!res.ok) { - throw data.detail; + throw new APIError(res.status, data.details ?? "") } return { diff --git a/frontend/lib/queries.ts b/frontend/lib/queries.ts index aac39c8..296daf7 100644 --- a/frontend/lib/queries.ts +++ b/frontend/lib/queries.ts @@ -5,17 +5,21 @@ import { Article, DailyArticleStats } from "@caviardeul/types"; import { API_URL } from "@caviardeul/utils/config"; import { decode } from "@caviardeul/utils/encryption"; -class APIError extends Error { +export class APIError extends Error { status: number; - details: any; + details: string; - constructor(status: number, details: any) { + constructor(status: number, details: string) { super("API Call error"); this.status = status; this.details = details; } } +export const isAPIError = (error: unknown): error is APIError => { + return !!(error && typeof error === 'object' && 'status' in error && 'details' in error) +} + export const saveGameScore = async ( article: Article, nbAttempts: number, @@ -69,8 +73,8 @@ const sendRequest = async (endpoint: string, { body }: any) => { if (!response.ok) { const status = response.status; - const details = await response.json(); - throw new APIError(status, details); + const data = await response.json(); + throw new APIError(status, data?.details ?? ""); } if (response.status === 204) { diff --git a/frontend/pages/404.tsx b/frontend/pages/404.tsx index e739a51..e328755 100644 --- a/frontend/pages/404.tsx +++ b/frontend/pages/404.tsx @@ -1,8 +1,8 @@ -import Error from "next/error"; import React from "react"; +import CustomError from "@caviardeul/components/utils/error"; const NotFoundPage: React.FC = () => { - return ; + return ; }; export default NotFoundPage; diff --git a/frontend/pages/archives/[articleId].tsx b/frontend/pages/archives/[articleId].tsx index 15de1c5..6eb82e2 100644 --- a/frontend/pages/archives/[articleId].tsx +++ b/frontend/pages/archives/[articleId].tsx @@ -6,10 +6,17 @@ import Game from "@caviardeul/components/game/game"; import { getEncodedArticle } from "@caviardeul/lib/article"; import { EncodedArticle } from "@caviardeul/types"; import { decodeArticle } from "@caviardeul/utils/encryption"; +import CustomError from "@caviardeul/components/utils/error"; +import {APIError, isAPIError} from "@caviardeul/lib/queries"; const ArchiveGame: NextPage<{ - encodedArticle: EncodedArticle; -}> = ({ encodedArticle, ...props }) => { + encodedArticle: EncodedArticle | null; + error?: APIError, +}> = ({ encodedArticle, error, ...props }) => { + if (!encodedArticle) { + return + } + const article = decodeArticle(encodedArticle); const title = `Caviardeul - Déchiffrez le Caviardeul n°${article.articleId}`; return ( @@ -32,11 +39,17 @@ export const getServerSideProps: GetServerSideProps = async ({ }) => { const { userId } = req.cookies; const articleId = parseInt(params?.articleId as string); - let encodedArticle; + let encodedArticle = null try { encodedArticle = await getEncodedArticle(articleId, false, userId); } catch (error) { - return { notFound: true }; + let apiError + if (isAPIError(error)) { + apiError = error + } else { + apiError = new APIError(500, "Une erreur est survenue") + } + return {props: {encodedArticle, error: {status: apiError.status, details: apiError.details}}} } const { userScore } = encodedArticle; diff --git a/frontend/pages/index.tsx b/frontend/pages/index.tsx index fa885f5..bda19f4 100644 --- a/frontend/pages/index.tsx +++ b/frontend/pages/index.tsx @@ -1,38 +1,48 @@ import type { GetServerSideProps, NextPage } from "next"; -import Error from "next/error"; import React from "react"; import Game from "@caviardeul/components/game/game"; import { getEncodedArticle } from "@caviardeul/lib/article"; import { EncodedArticle } from "@caviardeul/types"; import { decodeArticle } from "@caviardeul/utils/encryption"; +import CustomError from "@caviardeul/components/utils/error"; +import {APIError, isAPIError} from "@caviardeul/lib/queries"; const Home: NextPage<{ encodedArticle: EncodedArticle | null; userScore?: { nbAttempts: number; nbCorrect: number }; -}> = ({ encodedArticle, userScore }) => { - const dailyArticle = encodedArticle ? decodeArticle(encodedArticle) : null; - return dailyArticle ? ( + error?: APIError, +}> = ({ encodedArticle, userScore, error } +) => { + if (!encodedArticle) { + return + } + + const article = decodeArticle(encodedArticle); + return ( - ) : ( - - ); + ) }; export default Home; export const getServerSideProps: GetServerSideProps = async ({ req }) => { const { userId } = req.cookies; - let encodedArticle; + let encodedArticle = null; try { encodedArticle = await getEncodedArticle(undefined, undefined, userId); } catch (error) { - console.log(error); - return { props: { encodedArticle: null } }; + let apiError + if (isAPIError(error)) { + apiError = error + } else { + apiError = new APIError(500, "Une erreur est survenue") + } + return {props: {encodedArticle, error: {status: apiError.status, details: apiError.details}}} } const { userScore } = encodedArticle; diff --git a/frontend/styles/style.scss b/frontend/styles/style.scss index ed063fc..ca1386a 100644 --- a/frontend/styles/style.scss +++ b/frontend/styles/style.scss @@ -384,6 +384,10 @@ main { flex-flow: row; justify-content: center; height: calc(100vh - #{$navbarHeight + $footerHeight}); + + > div { + height: auto !important; + } } .left-container {