Skip to content

Commit

Permalink
fix(proof): remove is_public flag (#267)
Browse files Browse the repository at this point in the history
  • Loading branch information
raphael0202 authored Apr 9, 2024
1 parent 4c8d9b3 commit 0e300c8
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 139 deletions.
20 changes: 0 additions & 20 deletions app/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,23 +92,3 @@ def get_current_user(
detail="Invalid authentication credentials",
headers={"WWW-Authenticate": "Bearer"},
)


def get_current_user_optional(
token: Annotated[str, Depends(oauth2_scheme_no_error)],
db: Session = Depends(get_db),
) -> User | None:
"""Get the current user if authenticated, None otherwise.
This function is used as a dependency in endpoints that require
authentication, but where the user is optional.
:param token: the authentication token
:param db: the database session
:return: the current user if authenticated, None otherwise
"""
if token and "__U" in token:
session = crud.get_session_by_token(db, token=token)
if session:
return crud.update_session_last_used_field(db, session=session).user
return None
3 changes: 0 additions & 3 deletions app/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ def create_proof(
mimetype: str,
type: ProofTypeEnum,
user: UserCreate,
is_public: bool = True,
price_count: int = 0,
) -> Proof:
"""Create a proof in the database.
Expand All @@ -390,15 +389,13 @@ def create_proof(
:param file_path: the path to the file
:param mimetype: the mimetype of the file
:param user: the user who uploaded the file
:param is_public: whether the proof is public or not
:return: the created proof
"""
db_proof = Proof(
file_path=file_path,
mimetype=mimetype,
type=type,
owner=user.user_id,
is_public=is_public,
price_count=price_count,
)
db.add(db_proof)
Expand Down
4 changes: 0 additions & 4 deletions app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ class Proof(Base):
mimetype = mapped_column(String, index=True)

type: Mapped[ProofTypeEnum] = mapped_column(ChoiceType(ProofTypeEnum))
is_public = mapped_column(
Boolean, nullable=False, server_default="true", index=True
)

prices: Mapped[list["Price"]] = relationship(back_populates="proof")
price_count: Mapped[int] = mapped_column(
Integer, nullable=False, server_default="0", index=True
Expand Down
48 changes: 2 additions & 46 deletions app/routers/prices.py
Original file line number Diff line number Diff line change
@@ -1,70 +1,26 @@
import functools

from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status
from fastapi_filter import FilterDepends
from fastapi_pagination import Page
from fastapi_pagination.ext.sqlalchemy import paginate
from sqlalchemy.orm import Session

from app import crud, schemas, tasks
from app.auth import get_current_user, get_current_user_optional
from app.auth import get_current_user
from app.db import get_db
from app.models import Price
from app.schemas import PriceFullWithRelations

router = APIRouter(prefix="/prices")


def price_transformer(
prices: list[PriceFullWithRelations], current_user: schemas.UserCreate | None = None
) -> list[PriceFullWithRelations]:
"""Transformer function used to remove the file_path of private proofs.
If current_user is None, the file_path is removed for all proofs that are
not public. Otherwise, the file_path is removed for all proofs that are not
public and do not belong to the current user or is not a moderator.
:param prices: the list of prices to transform
:param current_user: the current user, if authenticated
:return: the transformed list of prices
"""
for price in prices:
if (
current_user is None
and price.proof is not None
and price.proof.is_public is False
):
price.proof.file_path = None
elif (
price.proof
and price.proof.is_public is False
and (
not current_user
or (
current_user
and (price.proof.owner != current_user.user_id)
and not current_user.is_moderator
)
)
):
price.proof.file_path = None
return prices


@router.get(
"",
response_model=Page[schemas.PriceFullWithRelations],
)
def get_prices(
filters: schemas.PriceFilter = FilterDepends(schemas.PriceFilter),
db: Session = Depends(get_db),
current_user: schemas.UserCreate | None = Depends(get_current_user_optional),
) -> list[Price]:
return paginate(
db,
crud.get_prices_query(filters=filters),
transformer=functools.partial(price_transformer, current_user=current_user),
)
return paginate(db, crud.get_prices_query(filters=filters))


@router.post(
Expand Down
14 changes: 1 addition & 13 deletions app/routers/proofs.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ def get_user_proofs(
def upload_proof(
file: UploadFile,
type: Annotated[ProofTypeEnum, Form(description="The type of the proof")],
is_public: bool = Form(
default=True,
description="if true, the proof is public and is included in the API response. "
"Set false only for RECEIPT proofs that contain personal information.",
),
current_user: schemas.UserCreate = Depends(get_current_user),
db: Session = Depends(get_db),
) -> Proof:
Expand All @@ -55,15 +50,8 @@ def upload_proof(
This endpoint requires authentication.
"""
if not is_public and type is not ProofTypeEnum.RECEIPT:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail="Only receipts can be private",
)
file_path, mimetype = crud.create_proof_file(file)
db_proof = crud.create_proof(
db, file_path, mimetype, type=type, user=current_user, is_public=is_public
)
db_proof = crud.create_proof(db, file_path, mimetype, type=type, user=current_user)
return db_proof


Expand Down
11 changes: 1 addition & 10 deletions app/schemas.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import datetime
from typing import Annotated, Optional
from typing import Optional

from fastapi_filter.contrib.sqlalchemy import Filter
from openfoodfacts import Flavor
Expand Down Expand Up @@ -169,14 +169,6 @@ class ProofFull(BaseModel):
mimetype: str
type: ProofTypeEnum | None = None
owner: str
is_public: Annotated[
bool,
Field(
default=True,
description="if true, the proof is public and is included in the API response. "
"Set false only if the proof contains personal information.",
),
]
price_count: int = Field(
description="number of prices for this proof.", examples=[15], default=0
)
Expand All @@ -185,7 +177,6 @@ class ProofFull(BaseModel):

class ProofBasicUpdatableFields(BaseModel):
type: ProofTypeEnum | None = None
is_public: bool | None = None

class Config:
extra = "forbid"
Expand Down
56 changes: 13 additions & 43 deletions tests/integration/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ def test_create_price(db_session, user_session: SessionModel, clean_prices):

def test_create_price_moderator(db_session, user_session, user_session_1, clean_prices):
crud.update_user_moderator(db_session, USER_1.user_id, False)
proof = crud.create_proof(
db_session, "/", " ", "PRICE_TAG", user_session.user, True
)
proof = crud.create_proof(db_session, "/", " ", "PRICE_TAG", user_session.user)

# user_1 is not moderator, fails to create a price with proof not owned
assert not user_session_1.user.is_moderator
Expand Down Expand Up @@ -524,10 +522,10 @@ def test_get_prices_with_proofs(
db_session, user_session: SessionModel, user_session_1: SessionModel, clean_prices
):
price_tag_proof = crud.create_proof(
db_session, "/", " ", "PRICE_TAG", user_session.user, is_public=True
db_session, "/", " ", "PRICE_TAG", user_session.user
)
receipt_proof = crud.create_proof(
db_session, "/", " ", "PRICE_TAG", user_session.user, is_public=False
db_session, "/", " ", "PRICE_TAG", user_session.user
)
crud.create_price(db_session, PRICE_1, user_session.user)
crud.create_price(
Expand All @@ -544,7 +542,7 @@ def test_get_prices_with_proofs(
# anonymous
response = client.get("/api/v1/prices")
assert response.json()["items"][0]["proof"]["file_path"] is not None
assert response.json()["items"][1]["proof"]["file_path"] is None # not public
assert response.json()["items"][1]["proof"]["file_path"] is not None
assert response.json()["items"][2]["proof"] is None

# authenticated but not owner nor moderator
Expand All @@ -553,9 +551,7 @@ def test_get_prices_with_proofs(
"/api/v1/prices", headers={"Authorization": f"Bearer {user_session_1.token}"}
)
assert response.json()["items"][0]["proof"]["file_path"] is not None
assert (
response.json()["items"][1]["proof"]["file_path"] is None
) # not public, not owner
assert response.json()["items"][1]["proof"]["file_path"] is not None
assert response.json()["items"][2]["proof"] is None

# authenticated and owner
Expand All @@ -574,9 +570,7 @@ def test_get_prices_with_proofs(
"/api/v1/prices", headers={"Authorization": f"Bearer {user_session_1.token}"}
)
assert response.json()["items"][0]["proof"]["file_path"] is not None
assert (
response.json()["items"][1]["proof"]["file_path"] is not None
) # not public, not owner, but moderator
assert response.json()["items"][1]["proof"]["file_path"] is not None
assert response.json()["items"][2]["proof"] is None


Expand Down Expand Up @@ -812,15 +806,6 @@ def test_create_proof(user_session: SessionModel, clean_proofs):
)
assert response.status_code == 422

# Check that is_public = False is not allowed for types other than RECEIPT
response = client.post(
"/api/v1/proofs/upload",
files={"file": ("filename", (io.BytesIO(b"test")), "image/webp")},
data={"type": "PRICE_TAG", "is_public": "false"},
headers={"Authorization": f"Bearer {user_session.token}"},
)
assert response.status_code == 422

# with authentication and no validation error
response = client.post(
"/api/v1/proofs/upload",
Expand All @@ -830,11 +815,10 @@ def test_create_proof(user_session: SessionModel, clean_proofs):
)
assert response.status_code == 201

# with authentication and is_public = False
response = client.post(
"/api/v1/proofs/upload",
files={"file": ("filename", (io.BytesIO(b"test")), "image/webp")},
data={"type": "RECEIPT", "is_public": "false"},
data={"type": "RECEIPT"},
headers={"Authorization": f"Bearer {user_session.token}"},
)
assert response.status_code == 201
Expand Down Expand Up @@ -866,7 +850,6 @@ def test_get_proofs(user_session: SessionModel):
"type",
"owner",
"created",
"is_public",
"price_count",
}

Expand All @@ -876,7 +859,6 @@ def test_get_proofs(user_session: SessionModel):
assert item["file_path"].endswith(".webp")
assert item["type"] == ("PRICE_TAG" if i == 0 else "RECEIPT")
assert item["owner"] == "user"
assert item["is_public"] == (True if i == 0 else False)
assert item["price_count"] == 0


Expand Down Expand Up @@ -915,11 +897,9 @@ def test_get_proofs_filters(db_session, user_session: SessionModel):
def test_get_proof(
db_session, user_session: SessionModel, user_session_1: SessionModel, clean_proofs
):
proof_user = crud.create_proof(
db_session, "/", " ", "PRICE_TAG", user_session.user, True
)
proof_user = crud.create_proof(db_session, "/", " ", "PRICE_TAG", user_session.user)
proof_user_1 = crud.create_proof(
db_session, "/", " ", "RECEIPT", user_session_1.user, True
db_session, "/", " ", "RECEIPT", user_session_1.user
)

# get without auth
Expand Down Expand Up @@ -970,7 +950,7 @@ def test_update_proof(
assert response.status_code == 201
proof = crud.get_proof_by_id(db_session, response.json().get("id"))

PROOF_UPDATE_PARTIAL = {"is_public": False}
PROOF_UPDATE_PARTIAL = {"type": "RECEIPT"}
# without authentication
response = client.patch(f"/api/v1/proofs/{proof.id}")
assert response.status_code == 401
Expand All @@ -996,18 +976,8 @@ def test_update_proof(
json=jsonable_encoder(PROOF_UPDATE_PARTIAL),
)
assert response.status_code == 200
assert response.json()["is_public"] is False
assert response.json()["type"] == proof.type.value
# with authentication and proof owner more fields
PROOF_UPDATE_PARTIAL_MORE = {**PROOF_UPDATE_PARTIAL, "type": "RECEIPT"}
response = client.patch(
f"/api/v1/proofs/{proof.id}",
headers={"Authorization": f"Bearer {user_session.token}"},
json=jsonable_encoder(PROOF_UPDATE_PARTIAL_MORE),
)
assert response.status_code == 200
assert response.json()["is_public"] is False
assert response.json()["type"] != proof.type.value
assert response.json()["type"] == "RECEIPT"

# with authentication and proof owner but extra fields
PROOF_UPDATE_PARTIAL_WRONG = {**PROOF_UPDATE_PARTIAL, "owner": 1}
response = client.patch(
Expand All @@ -1031,7 +1001,7 @@ def test_update_proof_moderator(
assert response.status_code == 201
proof = crud.get_proof_by_id(db_session, response.json().get("id"))

PROOF_UPDATE_PARTIAL = {"is_public": False}
PROOF_UPDATE_PARTIAL = {"type": "RECEIPT"}

# user_1 is moderator, not owner
crud.update_user_moderator(db_session, USER_1.user_id, True)
Expand Down

0 comments on commit 0e300c8

Please sign in to comment.