From ac23cb2863e92f1740b23359718337e4c787c967 Mon Sep 17 00:00:00 2001
From: Jesse Mortenson <jessemortenson@gmail.com>
Date: Fri, 26 Jan 2024 18:15:10 -0600
Subject: [PATCH 1/4] Bills: support bill action related entities in output

---
 README.md              | 28 ++++++++++++++++++++++++++--
 api/bills.py           |  1 +
 api/db/models/bills.py |  9 +++++++--
 api/schemas.py         | 11 +++++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index be3c6f0..4bda677 100644
--- a/README.md
+++ b/README.md
@@ -27,7 +27,8 @@ See [infrastructure repo](https://github.com/openstates/infrastructure#api-resta
 
 ## Running locally
 
-To run locally, you first need to have a running local database [following these instructions](https://docs.openstates.org/contributing/local-database/)
+To run locally, you first need to have a running local
+database [following these instructions](https://docs.openstates.org/contributing/local-database/)
 
 You also need to have a redis instance running. That can be run via the docker-compose config included in this package
 by running `docker compose up redis`
@@ -44,4 +45,27 @@ DATABASE_URL=postgresql://openstates:openstates@localhost:5405/openstatesorg poe
 
 To test out hitting the API, there will need to be a user account + profile entry with an API key in the local DB. The
 scripts involved in the above-mentioned instructions (openstates.org repo init DB) should result in the creation of an
-API key called `testkey` that can be used for local testing.
\ No newline at end of file
+API key called `testkey` that can be used for local testing.
+
+## Architecture
+
+Components of this FastAPI application:
+
+* Routes, found in the `api/` folder, such as `api/bills.py`. These expose HTTP API routes and contain the business
+  logic executed when a request hits that route.
+* SQL Alchemy models, found in the `api/db/models` folder, such as `api/db/models/bills.py` that define the data models
+  used by business logic to query data.
+* Pydantic schemas, found in the `api/schemas.py` folder, which define how data from the database is transformed into
+  output that is returned by business logic to the client.
+* Pagination logic, found in `api/pagination.py` and in route files, provides a semi-magic way for business logic to
+  use SQL Alchemy models to manage things like includes, pagination of results, etc..
+
+If you want to make a change, such as adding a related entity to output on an endpoint, you likely need to make changes
+to:
+
+* The Model file: add the new entity as a model, and a relationship property on the entity that is related to it. This
+  informs which columns are selected via which join logic.
+* The Pydantic schema: add the entity and fields to the output schema. Even if the Model changes are correct, the data
+  will not show up in API output unless it is in the schema.
+* The relevant Pagination object in the route file: you may need to add to `include_map_overrides` to tell the
+  pagination system that sub-entities should be fetched when an include is requested.
\ No newline at end of file
diff --git a/api/bills.py b/api/bills.py
index 1e0939d..259809d 100644
--- a/api/bills.py
+++ b/api/bills.py
@@ -49,6 +49,7 @@ class BillPagination(Pagination):
             "votes.sources",
             "votes.votes.voter",
         ],
+        BillInclude.actions: ["actions.related_entities"]
     }
     max_per_page = 20
 
diff --git a/api/db/models/bills.py b/api/db/models/bills.py
index 7c1f89f..b41644f 100644
--- a/api/db/models/bills.py
+++ b/api/db/models/bills.py
@@ -6,7 +6,7 @@
 from .. import Base
 from .common import PrimaryUUID, LinkBase, RelatedEntityBase
 from .jurisdiction import LegislativeSession
-from .people_orgs import Organization
+from .people_orgs import Organization, Person
 
 
 @lru_cache(100)
@@ -107,13 +107,18 @@ class BillAction(BillRelatedBase, Base):
     date = Column(String)
     classification = Column(ARRAY(Text), default=list)
     order = Column(Integer)
+    related_entities = relationship("BillActionRelatedEntity", back_populates="action")
 
 
 class BillActionRelatedEntity(RelatedEntityBase, Base):
     __tablename__ = "opencivicdata_billactionrelatedentity"
 
     action_id = Column(UUID(as_uuid=True), ForeignKey(BillAction.id))
-    action = relationship(BillAction)
+    action = relationship(BillAction, back_populates="related_entities")
+    name = Column(String)
+    entity_type = Column(String)
+    organization_id = Column(String, ForeignKey(Organization.id))
+    person_id = Column(String, ForeignKey(Person.id))
 
 
 class RelatedBill(PrimaryUUID, Base):
diff --git a/api/schemas.py b/api/schemas.py
index 7a89d8e..27bf136 100644
--- a/api/schemas.py
+++ b/api/schemas.py
@@ -256,6 +256,14 @@ class Config:
         orm_mode = True
 
 
+class BillActionRelatedEntity(BaseModel):
+    name: str = Field(..., example="Senate Committee of the Whole")
+    entity_type: str = Field(..., example="organization")
+
+    class Config:
+        orm_mode = True
+
+
 class BillAction(BaseModel):
     organization: Organization
     description: str = Field(..., example="Passed 1st Reading")
@@ -263,6 +271,7 @@ class BillAction(BaseModel):
     # TODO: enumerate billaction classifiers
     classification: List[str] = Field(..., example=["passed"])
     order: int
+    related_entities: Optional[List[BillActionRelatedEntity]]
 
     class Config:
         orm_mode = True
@@ -271,6 +280,8 @@ class Config:
 class BillDocumentLink(BaseModel):
     url: str = Field(..., example="https://example.com/doc.pdf")
     media_type: str = Field(..., example="application/pdf")
+    organization_id: str = Field(..., example="ocd-organization/fce467b7-470b-41c2-be23-0ed00804b512")
+    person_id: str = Field(..., example="ocd-person/5a327e3b-ae91-4279-ae17-9214aad32fa9")
 
     class Config:
         orm_mode = True

From cedeed730f96e4cd675585cc43a51cc0531a4b12 Mon Sep 17 00:00:00 2001
From: Jesse Mortenson <jessemortenson@gmail.com>
Date: Fri, 26 Jan 2024 18:18:35 -0600
Subject: [PATCH 2/4] Fix linting

---
 api/bills.py   | 2 +-
 api/schemas.py | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/api/bills.py b/api/bills.py
index 259809d..9723792 100644
--- a/api/bills.py
+++ b/api/bills.py
@@ -49,7 +49,7 @@ class BillPagination(Pagination):
             "votes.sources",
             "votes.votes.voter",
         ],
-        BillInclude.actions: ["actions.related_entities"]
+        BillInclude.actions: ["actions.related_entities"],
     }
     max_per_page = 20
 
diff --git a/api/schemas.py b/api/schemas.py
index 27bf136..37064d3 100644
--- a/api/schemas.py
+++ b/api/schemas.py
@@ -280,8 +280,12 @@ class Config:
 class BillDocumentLink(BaseModel):
     url: str = Field(..., example="https://example.com/doc.pdf")
     media_type: str = Field(..., example="application/pdf")
-    organization_id: str = Field(..., example="ocd-organization/fce467b7-470b-41c2-be23-0ed00804b512")
-    person_id: str = Field(..., example="ocd-person/5a327e3b-ae91-4279-ae17-9214aad32fa9")
+    organization_id: str = Field(
+        ..., example="ocd-organization/fce467b7-470b-41c2-be23-0ed00804b512"
+    )
+    person_id: str = Field(
+        ..., example="ocd-person/5a327e3b-ae91-4279-ae17-9214aad32fa9"
+    )
 
     class Config:
         orm_mode = True

From edbf970da3323022bf4ce2cf00ec05bd977115fc Mon Sep 17 00:00:00 2001
From: Jesse Mortenson <jessemortenson@gmail.com>
Date: Mon, 29 Jan 2024 12:35:37 -0600
Subject: [PATCH 3/4] Bill actions: fix missing sub-entity in pagination
 mapping

Also fix test expectation for queries, and add test intructions to README
---
 README.md               | 13 ++++++++++++-
 api/bills.py            |  2 +-
 api/tests/test_bills.py |  2 +-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index 4bda677..7ebffc4 100644
--- a/README.md
+++ b/README.md
@@ -47,6 +47,15 @@ To test out hitting the API, there will need to be a user account + profile entr
 scripts involved in the above-mentioned instructions (openstates.org repo init DB) should result in the creation of an
 API key called `testkey` that can be used for local testing.
 
+### Run tests
+
+* In addition to having the normal DB container running (as described above), you need to also start the `db-test`
+  service available in this project's `docker-compose.yml`: in this repo directory, run `docker compose up -d db-test`
+* Run all tests: `poetry run pytest` in the CLI; or in PyCharm configure a python test run targeting the
+  `pytest` module
+* Run an individual test: add an argument to either of the above specifying the test file and function name, eg
+  `api/tests/test_bills.py::test_bills_filter_by_jurisdiction_abbr`
+
 ## Architecture
 
 Components of this FastAPI application:
@@ -68,4 +77,6 @@ to:
 * The Pydantic schema: add the entity and fields to the output schema. Even if the Model changes are correct, the data
   will not show up in API output unless it is in the schema.
 * The relevant Pagination object in the route file: you may need to add to `include_map_overrides` to tell the
-  pagination system that sub-entities should be fetched when an include is requested.
\ No newline at end of file
+  pagination system that sub-entities should be fetched when an include is requested. If you add a sub-sub entity here,
+  such as "actions.related_entities" to the `BillPagination`, make sure to explicitly add the sub-entity as well:
+  "actions". Otherwise, additional queries will be generated to lazy-load the sub-entity.
\ No newline at end of file
diff --git a/api/bills.py b/api/bills.py
index 9723792..13ae5e0 100644
--- a/api/bills.py
+++ b/api/bills.py
@@ -49,7 +49,7 @@ class BillPagination(Pagination):
             "votes.sources",
             "votes.votes.voter",
         ],
-        BillInclude.actions: ["actions.related_entities"],
+        BillInclude.actions: ["actions", "actions.related_entities"],
     }
     max_per_page = 20
 
diff --git a/api/tests/test_bills.py b/api/tests/test_bills.py
index aecdffa..1275ab2 100644
--- a/api/tests/test_bills.py
+++ b/api/tests/test_bills.py
@@ -180,7 +180,7 @@ def test_bills_include_basics(client):
         "/bills?jurisdiction=ne&session=2020&include=sponsorships&include=abstracts"
         "&include=other_titles&include=other_identifiers&include=actions&include=sources"
     )
-    assert query_logger.count == 8
+    assert query_logger.count == 9
     assert response.status_code == 200
     for b in response.json()["results"]:
         assert len(b["sponsorships"]) == 2

From 094c2f9b6fdb27f7a192170aaccbb27e672897fd Mon Sep 17 00:00:00 2001
From: Jesse Mortenson <jessemortenson@gmail.com>
Date: Mon, 29 Jan 2024 13:55:07 -0600
Subject: [PATCH 4/4] Bills: fix related entity person/org fields

---
 api/schemas.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/api/schemas.py b/api/schemas.py
index 37064d3..b9373c0 100644
--- a/api/schemas.py
+++ b/api/schemas.py
@@ -259,6 +259,8 @@ class Config:
 class BillActionRelatedEntity(BaseModel):
     name: str = Field(..., example="Senate Committee of the Whole")
     entity_type: str = Field(..., example="organization")
+    organization: Optional[Organization] = Field(None, example=None)
+    person: Optional[CompactPerson]
 
     class Config:
         orm_mode = True
@@ -280,12 +282,6 @@ class Config:
 class BillDocumentLink(BaseModel):
     url: str = Field(..., example="https://example.com/doc.pdf")
     media_type: str = Field(..., example="application/pdf")
-    organization_id: str = Field(
-        ..., example="ocd-organization/fce467b7-470b-41c2-be23-0ed00804b512"
-    )
-    person_id: str = Field(
-        ..., example="ocd-person/5a327e3b-ae91-4279-ae17-9214aad32fa9"
-    )
 
     class Config:
         orm_mode = True