[Issue 642] Filter to only opportunities that are not drafts (#709)

[Issue 642] Filter to only opportunities that are not drafts --------- Co-authored-by: nava-platform-bot <[email protected]>
HHS · Nov 17, 2023 · 9f7c58f · 9f7c58f
1 parent 0caf864
commit 9f7c58f
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 51 deletions.
diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml
@@ -276,10 +276,6 @@ components:
           - C
           - E
           - O
-        is_draft:
-          type: boolean
-          description: Whether to search for draft claims
-          example: false
         sorting:
           $ref: '#/components/schemas/OpportunitySorting'
         paging:
@@ -317,10 +313,6 @@ components:
           - C
           - E
           - O
-        is_draft:
-          type: boolean
-          description: Whether the opportunity is in a draft status
-          example: false
         created_at:
           type: string
           format: date-time

diff --git a/api/src/api/opportunities/opportunity_schemas.py b/api/src/api/opportunities/opportunity_schemas.py
@@ -38,10 +38,6 @@ class OpportunitySchema(request_schema.OrderedSchema):
         },
     )
 
-    is_draft = fields.Boolean(
-        metadata={"description": "Whether the opportunity is in a draft status", "example": False}
-    )
-
     created_at = fields.DateTime(dump_only=True)
     updated_at = fields.DateTime(dump_only=True)
 
@@ -61,9 +57,6 @@ class OpportunitySearchSchema(request_schema.OrderedSchema):
             "example": OpportunityCategory.DISCRETIONARY,
         },
     )
-    is_draft = fields.Boolean(
-        metadata={"description": "Whether to search for draft claims", "example": False}
-    )
 
     sorting = fields.Nested(
         generate_sorting_schema(

diff --git a/api/src/services/opportunities/get_opportunities.py b/api/src/services/opportunities/get_opportunities.py
@@ -6,10 +6,11 @@
 
 
 def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity:
-    # TODO - issue-642 - figure out if we want to filter by is_draft here as well
-
+    # For now, only non-drafts can be fetched
     opportunity: Opportunity | None = db_session.execute(
-        select(Opportunity).where(Opportunity.opportunity_id == opportunity_id)
+        select(Opportunity)
+        .where(Opportunity.opportunity_id == opportunity_id)
+        .where(Opportunity.is_draft.is_(False))
     ).scalar_one_or_none()
 
     if opportunity is None:

diff --git a/api/src/services/opportunities/search_opportunities.py b/api/src/services/opportunities/search_opportunities.py
@@ -11,7 +11,6 @@
 
 class SearchOpportunityParams(PaginationParams):
     opportunity_title: str | None = None
-    is_draft: bool | None = None
     category: OpportunityCategory | None = None
 
 
@@ -22,8 +21,10 @@ def search_opportunities(
 
     sort_fn = asc if search_params.sorting.is_ascending else desc
 
-    stmt = select(Opportunity).order_by(
-        sort_fn(getattr(Opportunity, search_params.sorting.order_by))
+    stmt = (
+        select(Opportunity)
+        .order_by(sort_fn(getattr(Opportunity, search_params.sorting.order_by)))
+        .where(Opportunity.is_draft.is_(False))  # Only ever return non-drafts
     )
 
     if search_params.opportunity_title is not None:
@@ -32,9 +33,6 @@ def search_opportunities(
             Opportunity.opportunity_title.ilike(f"%{search_params.opportunity_title}%")
         )
 
-    if search_params.is_draft is not None:
-        stmt = stmt.where(Opportunity.is_draft == search_params.is_draft)
-
     if search_params.category is not None:
         stmt = stmt.where(Opportunity.category == search_params.category)
 
@@ -44,5 +42,4 @@ def search_opportunities(
     opportunities = paginator.page_at(page_offset=search_params.paging.page_offset)
     pagination_info = PaginationInfo.from_pagination_models(search_params, paginator)
 
-    # just creating a static pagination info for the moment until we hook up pagination
     return opportunities, pagination_info
diff --git a/api/tests/src/db/models/factories.py b/api/tests/src/db/models/factories.py
@@ -73,4 +73,4 @@ class Meta:
     opportunity_title = factory.LazyFunction(lambda: f"Research into {fake.job()} industry")
     agency = factory.Iterator(["US-ABC", "US-XYZ", "US-123"])
     category = factory.fuzzy.FuzzyChoice(OpportunityCategory)
-    is_draft = factory.Faker("boolean", chance_of_getting_true=50)
+    is_draft = False  # Because we filter out drafts, just default these to False
diff --git a/api/tests/src/route/test_opportunity_route.py b/api/tests/src/route/test_opportunity_route.py
@@ -22,7 +22,6 @@ def get_search_request(
     sort_direction: str = "descending",
     opportunity_title: str | None = None,
     category: str | None = None,
-    is_draft: bool | None = None,
 ):
     req = {
         "paging": {"page_offset": page_offset, "page_size": page_size},
@@ -35,9 +34,6 @@ def get_search_request(
     if category is not None:
         req["category"] = category
 
-    if is_draft is not None:
-        req["is_draft"] = is_draft
-
     return req
 
 
@@ -74,17 +70,18 @@ def setup_opportunities(enable_factory_create, truncate_opportunities):
     # Create a handful of opportunities for testing
     # Once we've built out the endpoint more, we'll probably want to make this more robust.
 
+    OpportunityFactory.create(opportunity_title="Find me abc", category=OpportunityCategory.EARMARK)
     OpportunityFactory.create(
-        opportunity_title="Find me abc", category=OpportunityCategory.EARMARK, is_draft=True
-    )
-    OpportunityFactory.create(
-        opportunity_title="Find me xyz", category=OpportunityCategory.CONTINUATION, is_draft=False
+        opportunity_title="Find me xyz", category=OpportunityCategory.CONTINUATION
     )
 
-    OpportunityFactory.create(category=OpportunityCategory.DISCRETIONARY, is_draft=True)
-    OpportunityFactory.create(category=OpportunityCategory.DISCRETIONARY, is_draft=False)
+    OpportunityFactory.create(category=OpportunityCategory.DISCRETIONARY)
+    OpportunityFactory.create(category=OpportunityCategory.DISCRETIONARY)
 
-    OpportunityFactory.create(category=OpportunityCategory.MANDATORY, is_draft=False)
+    OpportunityFactory.create(category=OpportunityCategory.MANDATORY)
+
+    # Add a few opportunities with is_draft=True which should never be found
+    OpportunityFactory.create_batch(size=10, is_draft=True)
 
 
 #####################################
@@ -138,37 +135,26 @@ def setup_opportunities(enable_factory_create, truncate_opportunities):
             get_search_request(category=OpportunityCategory.OTHER),
             SearchExpectedValues(total_pages=0, total_records=0, response_record_count=0),
         ),
-        # is_draft filter
-        (
-            get_search_request(is_draft=True),
-            SearchExpectedValues(total_pages=1, total_records=2, response_record_count=2),
-        ),
-        (
-            get_search_request(is_draft=False),
-            SearchExpectedValues(total_pages=1, total_records=3, response_record_count=3),
-        ),
         # A mix of filters
         (
             get_search_request(opportunity_title="find me", category=OpportunityCategory.EARMARK),
             SearchExpectedValues(total_pages=1, total_records=1, response_record_count=1),
         ),
         (
-            get_search_request(category=OpportunityCategory.DISCRETIONARY, is_draft=True),
-            SearchExpectedValues(total_pages=1, total_records=1, response_record_count=1),
+            get_search_request(category=OpportunityCategory.DISCRETIONARY),
+            SearchExpectedValues(total_pages=1, total_records=2, response_record_count=2),
         ),
         (
             get_search_request(
                 opportunity_title="find me",
                 category=OpportunityCategory.CONTINUATION,
-                is_draft=False,
             ),
             SearchExpectedValues(total_pages=1, total_records=1, response_record_count=1),
         ),
         (
             get_search_request(
                 opportunity_title="something else",
                 category=OpportunityCategory.OTHER,
-                is_draft=True,
             ),
             SearchExpectedValues(total_pages=0, total_records=0, response_record_count=0),
         ),
@@ -292,7 +278,6 @@ def test_opportunity_search_paging_and_sorting_200(
         ),
         (get_search_request(opportunity_title={}), {"opportunity_title": ["Not a valid string."]}),
         (get_search_request(category="X"), {"category": ["Must be one of: D, M, C, E, O."]}),
-        (get_search_request(is_draft="hello"), {"is_draft": ["Not a valid boolean."]}),
     ],
 )
 def test_opportunity_search_invalid_request_422(
@@ -368,6 +353,20 @@ def test_get_opportunity_not_found_404(client, api_auth_token, truncate_opportun
     assert resp.get_json()["message"] == "Could not find Opportunity with ID 1"
 
 
+def test_get_opportunity_not_found_is_draft_404(client, api_auth_token, enable_factory_create):
+    # The endpoint won't return drafts, so this'll be a 404 despite existing
+    opportunity = OpportunityFactory.create(is_draft=True)
+
+    resp = client.get(
+        f"/v1/opportunities/{opportunity.opportunity_id}", headers={"X-Auth": api_auth_token}
+    )
+    assert resp.status_code == 404
+    assert (
+        resp.get_json()["message"]
+        == f"Could not find Opportunity with ID {opportunity.opportunity_id}"
+    )
+
+
 def test_get_opportunity_invalid_id_404(client, api_auth_token):
     # with how the route naming resolves, this won't be an invalid request, but instead a 404
     resp = client.get("/v1/opportunities/text", headers={"X-Auth": api_auth_token})