Skip to content

Commit 79ec98d

Browse files
PGijsbersPGijsbers
authored
Move review POST endpoint to review section (#473)
* Move review POST endpoint to review section * Minor changes for consistency
1 parent 526038c commit 79ec98d

File tree

3 files changed

+73
-80
lines changed

3 files changed

+73
-80
lines changed

‎src/routers/resource_router.py‎

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,6 @@ def create(self, url_prefix: str) -> APIRouter:
150150
description=f"Retract a {self.resource_name}, setting its status to 'draft'.",
151151
**default_kwargs,
152152
)
153-
router.add_api_route(
154-
path=f"{url_prefix}/{self.resource_name_plural}/review/{version}",
155-
methods={"POST"},
156-
endpoint=self.get_review_func(),
157-
name=self.resource_name,
158-
description=f"Review a {self.resource_name}. Only for reviewers.",
159-
**default_kwargs,
160-
)
161153
router.add_api_route(
162154
path=f"{url_prefix}/{self.resource_name_plural}/{version}",
163155
methods={"POST"},
@@ -624,71 +616,6 @@ def retract_resource(
624616

625617
return retract_resource
626618

627-
def get_review_func(self):
628-
"""Return a function that can be used to review a single resource."""
629-
630-
def review_resource(
631-
review: ReviewCreate,
632-
user: KeycloakUser = Depends(get_user_or_raise),
633-
):
634-
if "reviewer" not in user.roles:
635-
raise HTTPException(
636-
status_code=status.HTTP_403_FORBIDDEN,
637-
detail="You must have reviewing privileges to use this endpoint.",
638-
)
639-
640-
with DbSession() as session:
641-
query = select(Submission).where(
642-
Submission.identifier == review.submission_identifier
643-
)
644-
submission = session.scalars(query).first()
645-
if submission is None:
646-
raise HTTPException(
647-
status_code=status.HTTP_400_BAD_REQUEST,
648-
detail=f"No review with identifier {review.submission_identifier} found.",
649-
)
650-
if not submission.is_pending:
651-
raise HTTPException(
652-
status_code=status.HTTP_400_BAD_REQUEST,
653-
detail="Review is no longer pending, no new decision may be made.",
654-
)
655-
register_user(user, session)
656-
657-
resource = self._retrieve_resource(
658-
identifier=submission.aiod_entry_identifier,
659-
session=session,
660-
is_entry_identifier=True,
661-
) # type: ignore
662-
if user_can_administer(user, resource.aiod_entry):
663-
raise HTTPException(
664-
status_code=status.HTTP_403_FORBIDDEN,
665-
detail="You do not have permission to review your own assets.",
666-
)
667-
668-
if review.decision == Decision.ACCEPTED:
669-
new_status = EntryStatus.PUBLISHED
670-
else:
671-
new_status = EntryStatus.DRAFT
672-
resource.aiod_entry.status = new_status
673-
674-
review = Review(
675-
reviewer_identifier=user._subject_identifier,
676-
comment=review.comment,
677-
decision=review.decision,
678-
submission_identifier=submission.identifier,
679-
)
680-
session.add(review)
681-
session.commit()
682-
return self._wrap_with_headers(
683-
{
684-
"review_identifier": review.identifier,
685-
"submission_identifier": submission.identifier,
686-
"decision": review.decision,
687-
}
688-
)
689-
690-
return review_resource
691-
692619
def _retrieve_resource(
693620
self,
694621
session: Session,

‎src/routers/review_router.py‎

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
import enum
22
from http import HTTPStatus
3-
from typing import Sequence, Literal
3+
from typing import Sequence, Literal, cast
44

55
from fastapi import APIRouter, HTTPException, Depends
6-
from sqlmodel import select
6+
from sqlmodel import select, Session
7+
from starlette import status
78

9+
from authentication import KeycloakUser, get_user_or_raise
10+
from database.authorization import register_user, user_can_administer
811
from database.session import DbSession, get_session
9-
from database.review import Submission, Review, SubmissionView, SubmissionBase
12+
from database.review import (
13+
Submission,
14+
Review,
15+
SubmissionView,
16+
SubmissionBase,
17+
ReviewCreate,
18+
Decision,
19+
)
20+
from database.model.concept.aiod_entry import EntryStatus, AIoDEntryORM
1021

1122

1223
def create(url_prefix: str) -> APIRouter:
@@ -27,6 +38,13 @@ def create(url_prefix: str) -> APIRouter:
2738
response_model=SubmissionView,
2839
)(get_submission)
2940

41+
router.post(
42+
f"{url_prefix}/reviews/{version}",
43+
tags=["Reviewing"],
44+
description="Review an asset.",
45+
response_model=Review,
46+
)(_review_resource)
47+
3048
# Add MiddleWare which requires authentication as reviewer role
3149
return router
3250

@@ -82,3 +100,51 @@ def get_submission(identifier: int, session=Depends(get_session)) -> Submission:
82100
status_code=HTTPStatus.NOT_FOUND,
83101
detail=f"No submission with identifier {identifier} found.",
84102
)
103+
104+
105+
def _review_resource(
106+
review: ReviewCreate,
107+
user: KeycloakUser = Depends(get_user_or_raise),
108+
session: Session = Depends(get_session),
109+
):
110+
if "reviewer" not in user.roles:
111+
raise HTTPException(
112+
status_code=status.HTTP_403_FORBIDDEN,
113+
detail="You must have reviewing privileges to use this endpoint.",
114+
)
115+
116+
submission = session.get(Submission, review.submission_identifier)
117+
if submission is None:
118+
raise HTTPException(
119+
status_code=status.HTTP_400_BAD_REQUEST,
120+
detail=f"No review with identifier {review.submission_identifier} found.",
121+
)
122+
if not submission.is_pending:
123+
raise HTTPException(
124+
status_code=status.HTTP_400_BAD_REQUEST,
125+
detail="Review is no longer pending, no new decision may be made.",
126+
)
127+
register_user(user, session)
128+
129+
aiod_entry = cast(AIoDEntryORM, session.get(AIoDEntryORM, submission.aiod_entry_identifier))
130+
if user_can_administer(user, aiod_entry):
131+
raise HTTPException(
132+
status_code=status.HTTP_403_FORBIDDEN,
133+
detail="You do not have permission to review your own assets.",
134+
)
135+
136+
if review.decision == Decision.ACCEPTED:
137+
new_status = EntryStatus.PUBLISHED
138+
else:
139+
new_status = EntryStatus.DRAFT
140+
aiod_entry.status = new_status
141+
142+
review = Review(
143+
reviewer_identifier=user._subject_identifier,
144+
comment=review.comment,
145+
decision=review.decision,
146+
submission_identifier=submission.identifier,
147+
)
148+
session.add(review)
149+
session.commit()
150+
return review

‎src/tests/authorization/test_authorization.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ def test_only_reviewer_can_approve_submission(publication, client):
345345

346346
with logged_in_user(ALICE):
347347
response = client.post(
348-
"/publications/review/v1",
348+
"/reviews/v1",
349349
content=str(
350350
ReviewCreate(decision=Decision.ACCEPTED, submission_identifier=1, comment="").json()
351351
),
@@ -355,7 +355,7 @@ def test_only_reviewer_can_approve_submission(publication, client):
355355

356356
with logged_in_user(REVIEWER):
357357
response = client.post(
358-
"/publications/review/v1",
358+
"/reviews/v1",
359359
content=str(
360360
ReviewCreate(decision=Decision.ACCEPTED, submission_identifier=1, comment="").json()
361361
),
@@ -374,7 +374,7 @@ def test_reviewer_can_reject_submission(publication, client):
374374

375375
with logged_in_user(REVIEWER):
376376
response = client.post(
377-
"/publications/review/v1",
377+
"/reviews/v1",
378378
content=str(
379379
ReviewCreate(decision=Decision.REJECTED, submission_identifier=1, comment="").json()
380380
),
@@ -393,7 +393,7 @@ def test_reviewer_cannot_approve_own_submission(publication, client):
393393

394394
with logged_in_user(REVIEWER):
395395
response = client.post(
396-
"/publications/review/v1",
396+
"/reviews/v1",
397397
content=str(
398398
ReviewCreate(decision=Decision.ACCEPTED, submission_identifier=1, comment="").json()
399399
),

0 commit comments

Comments
 (0)