Skip to content

Commit

Permalink
[Issue #2371] Rename agency to agency_code in API + DB (#2566)
Browse files Browse the repository at this point in the history
## Summary
Fixes #2371

### Time to review: __10 mins__

## Changes proposed
High-level - renamed `agency` in the opportunity models to `agency_code`
- both at the API and DB layer

Remove agency name and agency code from the opportunity summary response
object (should not be used - the opportunity values are more accurate)

A lot of fixes for tests that were checking for the old name

Deleted an import-csv script that wasn't needed anymore (and would've
needed fixes)

## Context for reviewers
`agency` is way too generic of a name, and `agency_code` makes more
sense.

Agency info is also split across an opportunity and summary. Some of
this was duplicated (code and name) and we want to avoid confusion by
having these values duplicated.

## Additional information
Did a quick pass on the existing search UI, don't see anything having
broken yet. Query box, sorting, and filters all work, as well as the
agency itself being displayed in search. I also switched the frontend to
use v1 and didn't see any issues

<img width="1184" alt="Screenshot 2024-10-25 at 3 33 08 PM"
src="https://github.com/user-attachments/assets/ff0ad334-69dd-43da-a548-dcc5591e26c0">
<img width="1171" alt="Screenshot 2024-10-25 at 3 33 27 PM"
src="https://github.com/user-attachments/assets/569a4f73-cdba-45b8-94df-910a1841863f">

---------

Co-authored-by: nava-platform-bot <platform-admins@navapbc.com>
  • Loading branch information
chouinar and nava-platform-bot authored Nov 12, 2024
1 parent 56acd6d commit c5dd97f
Show file tree
Hide file tree
Showing 28 changed files with 127 additions and 222 deletions.
28 changes: 16 additions & 12 deletions api/openapi.generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1276,18 +1276,6 @@ components:
description: Additional information about the types of applicants that are
eligible
example: All types of domestic applicants are eligible to apply
agency_code:
type:
- string
- 'null'
description: The agency who owns the opportunity
example: US-ABC
agency_name:
type:
- string
- 'null'
description: The name of the agency who owns the opportunity
example: US Alphabetical Basic Corp
agency_phone_number:
type:
- string
Expand Down Expand Up @@ -1409,6 +1397,12 @@ components:
description: The title of the opportunity
example: Research into conservation techniques
agency:
type:
- string
- 'null'
description: 'DEPRECATED - use: agency_code'
example: US-ABC
agency_code:
type:
- string
- 'null'
Expand Down Expand Up @@ -1950,6 +1944,10 @@ components:
type: string
description: The agency who created the opportunity
example: US-ABC
agency_code:
type: string
description: The agency who created the opportunity
example: US-ABC
category:
description: The opportunity category
example: !!python/object/apply:src.constants.lookup_constants.OpportunityCategory
Expand Down Expand Up @@ -2093,6 +2091,12 @@ components:
description: The title of the opportunity
example: Research into conservation techniques
agency:
type:
- string
- 'null'
description: 'DEPRECATED - use: agency_code'
example: US-ABC
agency_code:
type:
- string
- 'null'
Expand Down
3 changes: 3 additions & 0 deletions api/src/api/opportunities_v0_1/opportunity_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ class OpportunityV01Schema(Schema):
agency = fields.String(
metadata={"description": "The agency who created the opportunity", "example": "US-ABC"}
)
agency_code = fields.String(
metadata={"description": "The agency who created the opportunity", "example": "US-ABC"}
)

category = fields.Enum(
OpportunityCategory,
Expand Down
20 changes: 6 additions & 14 deletions api/src/api/opportunities_v1/opportunity_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,6 @@ class OpportunitySummaryV1Schema(Schema):
},
)

agency_code = fields.String(
allow_none=True,
metadata={
"description": "The agency who owns the opportunity",
"example": "US-ABC",
},
)
agency_name = fields.String(
allow_none=True,
metadata={
"description": "The name of the agency who owns the opportunity",
"example": "US Alphabetical Basic Corp",
},
)
agency_phone_number = fields.String(
allow_none=True,
metadata={
Expand Down Expand Up @@ -263,7 +249,13 @@ class OpportunityV1Schema(Schema):
"example": "Research into conservation techniques",
},
)
# TODO - we'll want to remove this field in the future
# but need to make sure the frontend is not using it
agency = fields.String(
allow_none=True,
metadata={"description": "DEPRECATED - use: agency_code", "example": "US-ABC"},
)
agency_code = fields.String(
allow_none=True,
metadata={"description": "The agency who created the opportunity", "example": "US-ABC"},
)
Expand Down
2 changes: 1 addition & 1 deletion api/src/data_migration/transformation/transform_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def transform_opportunity(

target_opportunity.opportunity_number = source_opportunity.oppnumber
target_opportunity.opportunity_title = source_opportunity.opptitle
target_opportunity.agency = source_opportunity.owningagency
target_opportunity.agency_code = source_opportunity.owningagency
target_opportunity.category = transform_opportunity_category(source_opportunity.oppcategory)
target_opportunity.category_explanation = source_opportunity.category_explanation
target_opportunity.revision_number = source_opportunity.revision_number
Expand Down
39 changes: 39 additions & 0 deletions api/src/db/migrations/versions/2024_11_01_rename_agency_column.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""rename agency column
Revision ID: 3640e31e6a85
Revises: 8b96ade6f6a2
Create Date: 2024-11-01 12:57:11.887858
"""

from alembic import op

# revision identifiers, used by Alembic.
revision = "3640e31e6a85"
down_revision = "8b96ade6f6a2"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_index("opportunity_agency_idx", table_name="opportunity", schema="api")
op.alter_column("opportunity", "agency", new_column_name="agency_code", schema="api")
op.create_index(
op.f("opportunity_agency_code_idx"),
"opportunity",
["agency_code"],
unique=False,
schema="api",
)
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_index(op.f("opportunity_agency_code_idx"), table_name="opportunity", schema="api")
op.alter_column("opportunity", "agency_code", new_column_name="agency", schema="api")
op.create_index(
"opportunity_agency_idx", "opportunity", ["agency_code"], unique=False, schema="api"
)
# ### end Alembic commands ###
16 changes: 12 additions & 4 deletions api/src/db/models/opportunity_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ class Opportunity(ApiSchemaTable, TimestampMixin):

opportunity_number: Mapped[str | None]
opportunity_title: Mapped[str | None] = mapped_column(index=True)
agency_code: Mapped[str | None] = mapped_column(index=True)

agency: Mapped[str | None] = mapped_column(index=True)
@property
def agency(self) -> str | None:
# TODO - this is temporary until the frontend no longer needs this name
return self.agency_code

category: Mapped[OpportunityCategory | None] = mapped_column(
"opportunity_category_id",
Expand Down Expand Up @@ -75,7 +79,7 @@ class Opportunity(ApiSchemaTable, TimestampMixin):

agency_record: Mapped[Agency | None] = relationship(
Agency,
primaryjoin="Opportunity.agency == foreign(Agency.agency_code)",
primaryjoin="Opportunity.agency_code == foreign(Agency.agency_code)",
uselist=False,
viewonly=True,
)
Expand Down Expand Up @@ -182,8 +186,6 @@ class OpportunitySummary(ApiSchemaTable, TimestampMixin):
funding_category_description: Mapped[str | None]
applicant_eligibility_description: Mapped[str | None]

agency_code: Mapped[str | None]
agency_name: Mapped[str | None]
agency_phone_number: Mapped[str | None]
agency_contact_description: Mapped[str | None]
agency_email_address: Mapped[str | None]
Expand All @@ -198,6 +200,12 @@ class OpportunitySummary(ApiSchemaTable, TimestampMixin):
updated_by: Mapped[str | None]
created_by: Mapped[str | None]

# Do not use these agency fields, they're kept for now, but
# are simply copying behavior from the legacy system - prefer
# the same named values in the opportunity itself
agency_code: Mapped[str | None]
agency_name: Mapped[str | None]

link_funding_instruments: Mapped[list["LinkOpportunitySummaryFundingInstrument"]] = (
relationship(
back_populates="opportunity_summary", uselist=True, cascade="all, delete-orphan"
Expand Down
2 changes: 1 addition & 1 deletion api/src/pagination/paginator.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Paginator(Generic[T]):
stmt = select(User).order_by(desc("opportunity_id"))
# Add any filters
stmt = stmt.where(Opportunity.agency == "US-XYZ")
stmt = stmt.where(Opportunity.agency_code == "US-XYZ")
# Use the paginator to get a specific page (page 2 in this case)
paginator: Paginator[Opportunity] = Paginator(stmt, db_session, page_size=10)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def convert_transfer_opp_to_regular(transfer_opportunity: TransferTopportunity)
opportunity_id=transfer_opportunity.opportunity_id,
opportunity_number=transfer_opportunity.oppnumber,
opportunity_title=transfer_opportunity.opptitle,
agency=transfer_opportunity.owningagency,
agency_code=transfer_opportunity.owningagency,
category=opportunity_category,
category_explanation=transfer_opportunity.category_explanation,
is_draft=is_draft,
Expand Down
6 changes: 3 additions & 3 deletions api/src/services/opportunities_v0_1/search_opportunities.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _add_query_filters(stmt: Select[tuple[Any]], query: str | None) -> Select[tu
# Number partial match
Opportunity.opportunity_number.ilike(ilike_query),
# Agency (code) partial match
Opportunity.agency.ilike(ilike_query),
Opportunity.agency_code.ilike(ilike_query),
# Summary description partial match
OpportunitySummary.summary_description.ilike(ilike_query),
# assistance listing number matches exactly or program title partial match
Expand Down Expand Up @@ -143,7 +143,7 @@ def _add_filters(
# Note that we filter against the agency code in the opportunity, not in the summary
one_of_agencies = filters.agency.get("one_of")
if one_of_agencies:
stmt = stmt.where(Opportunity.agency.in_(one_of_agencies))
stmt = stmt.where(Opportunity.agency_code.in_(one_of_agencies))

return stmt

Expand Down Expand Up @@ -174,7 +174,7 @@ def _add_order_by(
# Need to add joins to the query stmt to order by field from opportunity summary
stmt = _join_stmt_to_current_summary(stmt)
case "agency_code":
field = Opportunity.agency
field = Opportunity.agency_code
case _:
# If this exception happens, it means our API schema
# allows for values we don't have implemented. This
Expand Down
4 changes: 2 additions & 2 deletions api/src/services/opportunities_v1/search_opportunities.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
"opportunity_title": "opportunity_title.keyword",
"post_date": "summary.post_date",
"close_date": "summary.close_date",
"agency_code": "agency.keyword",
"agency": "agency.keyword",
"agency_code": "agency_code.keyword",
"agency": "agency_code.keyword",
"opportunity_status": "opportunity_status.keyword",
"funding_instrument": "summary.funding_instruments.keyword",
"funding_category": "summary.funding_categories.keyword",
Expand Down
1 change: 0 additions & 1 deletion api/src/task/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

# import any of the other files so they get initialized and attached to the blueprint
import src.task.opportunities.set_current_opportunities_task # noqa: F401 E402 isort:skip
import src.task.opportunities.import_opportunity_csvs # noqa: F401 E402 isort:skip
import src.task.opportunities.export_opportunity_data_task # noqa: F401 E402 isort:skip

__all__ = ["task_blueprint"]
71 changes: 0 additions & 71 deletions api/src/task/opportunities/import_opportunity_csvs.py

This file was deleted.

2 changes: 1 addition & 1 deletion api/tests/lib/seed_local_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def _build_opportunities(db_session: db.Session, iterations: int, include_histor
{
"agency_id": 4,
"agency_code": "DOC-EDA",
"agency_name": "Agency for International Development",
"agency_name": "Economic Development Administration",
"top_level_agency_id": 3, # DOC
},
]
Expand Down
4 changes: 2 additions & 2 deletions api/tests/src/api/opportunities_v0_1/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def setup_opportunity(
opportunity = OpportunityFactory.create(
opportunity_id=opportunity_id,
no_current_summary=True,
agency=agency,
agency_code=agency,
is_draft=is_draft,
opportunity_title=opportunity_title,
opportunity_number=opportunity_number,
Expand Down Expand Up @@ -165,7 +165,7 @@ def validate_opportunity(db_opportunity: Opportunity, resp_opportunity: dict):
assert db_opportunity.opportunity_id == resp_opportunity["opportunity_id"]
assert db_opportunity.opportunity_number == resp_opportunity["opportunity_number"]
assert db_opportunity.opportunity_title == resp_opportunity["opportunity_title"]
assert db_opportunity.agency == resp_opportunity["agency"]
assert db_opportunity.agency_code == resp_opportunity["agency_code"]
assert db_opportunity.category == resp_opportunity["category"]
assert db_opportunity.category_explanation == resp_opportunity["category_explanation"]

Expand Down
4 changes: 1 addition & 3 deletions api/tests/src/api/opportunities_v1/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def validate_opportunity(db_opportunity: Opportunity, resp_opportunity: dict):
assert db_opportunity.opportunity_id == resp_opportunity["opportunity_id"]
assert db_opportunity.opportunity_number == resp_opportunity["opportunity_number"]
assert db_opportunity.opportunity_title == resp_opportunity["opportunity_title"]
assert db_opportunity.agency == resp_opportunity["agency"]
assert db_opportunity.agency_code == resp_opportunity["agency_code"]
assert db_opportunity.agency_name == resp_opportunity["agency_name"]
assert db_opportunity.category == resp_opportunity["category"]
assert db_opportunity.category_explanation == resp_opportunity["category_explanation"]
Expand Down Expand Up @@ -208,8 +208,6 @@ def validate_opportunity_summary(db_summary: OpportunitySummary, resp_summary: d
== resp_summary["applicant_eligibility_description"]
)

assert db_summary.agency_code == resp_summary["agency_code"]
assert db_summary.agency_name == resp_summary["agency_name"]
assert db_summary.agency_phone_number == resp_summary["agency_phone_number"]
assert db_summary.agency_contact_description == resp_summary["agency_contact_description"]
assert db_summary.agency_email_address == resp_summary["agency_email_address"]
Expand Down
Loading

0 comments on commit c5dd97f

Please sign in to comment.