test hygiene: null-deref guards in 2 endpoints, drop 2 dead schema fields, add CONTRIBUTING.md (#114)

Author: chai-inu

CONTRIBUTING.md

@@ -0,0 +1,17 @@
+# Contributing
+
+## Prod-first principle
+
+When prod code and the test suite disagree, the default assumption is **prod is the spec, tests are not**.
+
+In practice:
+
+1. **Don't modify prod to fit the test suite.** Follow good practice in prod code. If a test depends on prod doing something it shouldn't (e.g. the test expects a free-coin grant that prod no longer issues), the test is wrong — fix the test, not prod.
+
+2. **A prod change that breaks a test is not automatically a regression.** Analyze before reacting:
+   - Does the test exercise behavior the spec actually requires?
+   - Or was the test written against the old shape of prod code that no longer exists?
+
+   If the latter, rewrite or delete the test. Old tests are not contracts.
+
+This shapes how we land refactors: a refactor PR that rewrites or deletes affected tests is healthy. A refactor PR that leaves prod uglier to keep old tests green is not.

chafan_core/app/api/api_v1/endpoints/answers.py

@@ -168,6 +168,11 @@ def bump_views_counter(
     cached_layer: CachedLayer = Depends(deps.get_cached_layer),
 ) -> Any:
     answer = crud.answer.get_by_uuid(cached_layer.get_db(), uuid=uuid)
+    if answer is None:
+        raise HTTPException_(
+            status_code=400,
+            detail="The answer doesn't exist in the system.",
+        )
     view_counters.add_view_async(cached_layer, "answer", answer.id)
     return schemas.GenericResponse()
 

chafan_core/app/api/api_v1/endpoints/questions.py

@@ -90,6 +90,11 @@ def get_question(
     Get question in one of current_user's belonging sites.
     """
     question = cached_layer.get_question_by_uuid(uuid)
+    if question is None:
+        raise HTTPException_(
+            status_code=404,
+            detail="The question doesn't exist in the system.",
+        )
     if question.is_hidden:
         raise HTTPException_(
                 status_code=403,

chafan_core/app/cached_layer.py

@@ -656,7 +656,7 @@ def get_question_model_http(self, uuid: str) -> models.Question:
             )
         return question
 
-    def get_question_by_uuid(self, uuid: str, current_user_id:Optional[int]=None) -> models.Question:
+    def get_question_by_uuid(self, uuid: str, current_user_id:Optional[int]=None) -> Optional[models.Question]:
         question = crud.question.get_by_uuid(self.get_db(), uuid=uuid)
         if question is None:
             return None

chafan_core/app/schemas/article_archive.py

@@ -1,17 +1,14 @@
 import datetime
-from typing import List
 
 from pydantic import BaseModel
 
 from chafan_core.app.schemas.richtext import RichText
-from chafan_core.app.schemas.topic import Topic
 from chafan_core.utils.constants import editor_T
 
 
 class ArticleArchiveInDB(BaseModel):
     id: int
     title: str
-    topics: List[Topic] = []
     created_at: datetime.datetime
 
     # TODO: deprecate

chafan_core/app/schemas/user.py

@@ -115,7 +115,6 @@ class UserInDBBase(UserBase):
     about: Optional[str] = None
     created_at: datetime.datetime
     residency_topics: List[Topic]
-    profession_topic: Optional[Topic] = None
     remaining_coins: int
     personal_introduction: Optional[str] = None
     github_username: Optional[str] = None

chafan_core/tests/app/api/api_v1/test_questions.py

@@ -182,7 +182,6 @@ def test_get_question_success(
     assert db_question.uuid == response_data["uuid"]
 
 
-@pytest.mark.skip(reason="TODO: get_question endpoint doesn't handle None question before accessing is_hidden")
 def test_get_question_nonexistent(
     client: TestClient,
     db: Session,
@@ -193,7 +192,7 @@ def test_get_question_nonexistent(
         f"{settings.API_V1_STR}/questions/invalid-uuid",
         headers=normal_user_token_headers,
     )
-    assert r.status_code == 400
+    assert r.status_code == 404
 
     # Verify it doesn't exist in database
     db_question = crud.question.get_by_uuid(db, uuid="invalid-uuid")