Skip to content

Commit

Permalink
Merge pull request #3000 from HHS/OPS-1981/remove-incumbent
Browse files Browse the repository at this point in the history
Removing the Incumbent from the Agreement object and replacing it with Vendor
  • Loading branch information
rajohnson90 authored Oct 30, 2024
2 parents ce20a60 + f8e5fb5 commit e33adc5
Show file tree
Hide file tree
Showing 26 changed files with 108 additions and 117 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""Remove incumbent from contract agreement. Prefer to use Vendor instead
Revision ID: 6615ac7d4eea
Revises: af64d84afb2e
Create Date: 2024-10-28 19:47:50.822292+00:00
"""
from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = '6615ac7d4eea'
down_revision: Union[str, None] = 'af64d84afb2e'
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_constraint('contract_agreement_incumbent_id_fkey', 'contract_agreement', type_='foreignkey')
op.drop_column('contract_agreement', 'incumbent_id')
op.drop_column('contract_agreement_version', 'incumbent_id')
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.add_column('contract_agreement_version', sa.Column('incumbent_id', sa.INTEGER(), autoincrement=False, nullable=True))
op.add_column('contract_agreement', sa.Column('incumbent_id', sa.INTEGER(), autoincrement=False, nullable=True))
op.create_foreign_key('contract_agreement_incumbent_id_fkey', 'contract_agreement', 'vendor', ['incumbent_id'], ['id'])
# ### end Alembic commands ###
6 changes: 0 additions & 6 deletions backend/data_tools/data/agreements_and_blin_data.json5
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
awarding_entity_id: 4,
created_by: 503,
contract_number: "XXXX000000006",
incumbent_id: null,
vendor_id: 100,
delivered_status: false,
contract_type: "FIRM_FIXED_PRICE",
Expand All @@ -99,7 +98,6 @@
awarding_entity_id: 4,
created_by: 503,
contract_number: "XXXX000000007",
incumbent_id: null,
vendor_id: 101,
delivered_status: false,
contract_type: "TIME_AND_MATERIALS",
Expand All @@ -118,7 +116,6 @@
awarding_entity_id: 4,
created_by: 503,
contract_number: "XXXX000000008",
incumbent_id: null,
vendor_id: 101,
delivered_status: false,
contract_type: "LABOR_HOUR",
Expand All @@ -138,7 +135,6 @@
awarding_entity_id: 3,
created_by: 503,
contract_number: "XXXX000000009",
incumbent_id: null,
vendor_id: 102,
delivered_status: false,
contract_type: "LABOR_HOUR",
Expand Down Expand Up @@ -167,7 +163,6 @@
awarding_entity_id: 1,
created_by: 503,
contract_number: "XXXX000000001",
incumbent_id: 100,
vendor_id: 100,
delivered_status: false,
contract_type: "LABOR_HOUR",
Expand Down Expand Up @@ -204,7 +199,6 @@
awarding_entity_id: 3,
created_by: 503,
contract_number: "XXXX000000010",
incumbent_id: null,
vendor_id: 102,
delivered_status: false,
contract_type: "LABOR_HOUR",
Expand Down
1 change: 0 additions & 1 deletion backend/data_tools/data/first_contract_data.json5
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
awarding_entity_id: 1,
created_by: 503,
contract_number: "XXXX000000001",
incumbent_id: 100,
vendor_id: 100,
delivered_status: false,
contract_type: "LABOR_HOUR",
Expand Down
4 changes: 0 additions & 4 deletions backend/models/agreements.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,6 @@ class ContractAgreement(Agreement):

id: Mapped[int] = mapped_column(ForeignKey("agreement.id"), primary_key=True)
contract_number: Mapped[Optional[str]] = mapped_column(String)
incumbent_id: Mapped[Optional[int]] = mapped_column(ForeignKey("vendor.id"))
incumbent: Mapped[Optional["Vendor"]] = relationship(
"Vendor", foreign_keys=[incumbent_id]
)
vendor_id: Mapped[Optional[int]] = mapped_column(ForeignKey("vendor.id"))
vendor: Mapped[Optional["Vendor"]] = relationship(
"Vendor", foreign_keys=[vendor_id]
Expand Down
3 changes: 0 additions & 3 deletions backend/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3262,9 +3262,6 @@ components:
agreement_reason:
type: string
example: NEW_REQ
incumbent:
type: string
example: Previous Vendor
vendor:
type: string
example: Current Vendor
Expand Down
5 changes: 0 additions & 5 deletions backend/ops_api/ops/resources/agreements.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ def _create_agreement(self, data, agreement_cls):
# TODO: add_vendor is here temporarily until we have vendor management
# implemented in the frontend, i.e. the vendor is a drop-down instead
# of a text field
add_update_vendor(data, "incumbent")
add_update_vendor(data, "vendor")

new_agreement = agreement_cls(**data)
Expand Down Expand Up @@ -357,10 +356,6 @@ def update_data(agreement: Agreement, data: dict[str, Any]) -> None:
if isinstance(data[item], str):
add_update_vendor(data, "vendor", agreement)
changed = True
case "incumbent":
if isinstance(data[item], str):
add_update_vendor(data, "incumbent", agreement)
changed = True
case _:
if getattr(agreement, item) != data[item]:
setattr(agreement, item, data[item])
Expand Down
8 changes: 0 additions & 8 deletions backend/ops_api/ops/schemas/agreements.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,12 @@ class Vendor(Schema):
name = fields.String(required=True)


class Incumbent(Schema):
name = fields.String(required=True)


class AgreementData(Schema):
name = fields.String(required=True)
agreement_type = fields.Enum(AgreementType, required=True)
description = fields.String(allow_none=True)
product_service_code_id = fields.Integer(allow_none=True)
agreement_reason = fields.Enum(AgreementReason, allow_none=True)
incumbent = fields.String(allow_none=True)
project_officer_id = fields.Integer(allow_none=True)
team_members = fields.List(fields.Nested(TeamMembers), default=[], allow_none=True)
project_id = fields.Integer(allow_none=True)
Expand All @@ -33,7 +28,6 @@ class AgreementData(Schema):

class ContractAgreementData(AgreementData):
contract_number = fields.String(allow_none=True)
incumbent = fields.String(allow_none=True)
vendor = fields.String(allow_none=True)
delivered_status = fields.Bool(default=False)
contract_type = fields.Enum(ContractType, allow_none=True)
Expand Down Expand Up @@ -72,8 +66,6 @@ class AgreementResponse(AgreementData):

class ContractAgreementResponse(AgreementResponse):
contract_number = fields.String(allow_none=True)
incumbent = fields.Pluck("Incumbent", "name")
incumbent_id = fields.Integer(allow_none=True)
vendor_id = fields.Integer(allow_none=True)
vendor = fields.Pluck("Vendor", "name")
delivered_status = fields.Bool(default=False)
Expand Down
16 changes: 7 additions & 9 deletions backend/ops_api/ops/schemas/budget_line_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,20 @@ def validate_agreement_reason(self, data, **kwargs):
raise ValidationError("BLI's Agreement must have an AgreementReason when status is not DRAFT")

@validates_schema
def validate_agreement_reason_must_not_have_incumbent(self, data, **kwargs):
def validate_agreement_reason_must_not_have_vendor(self, data, **kwargs):
if self.status_is_changing_beyond_draft(data):
bli = self.get_current_budget_line_item()
if (
bli
and bli.agreement_id
and isinstance(bli.agreement, ContractAgreement) # only contracts have incumbents
and isinstance(bli.agreement, ContractAgreement) # only contracts have vendors
and bli.agreement.agreement_reason == AgreementReason.NEW_REQ
and bli.agreement.incumbent_id
and bli.agreement.vendor_id
):
raise ValidationError(
"BLI's Agreement cannot have an Incumbent if it has an Agreement Reason of NEW_REQ"
)
raise ValidationError("BLI's Agreement cannot have a Vendor if it has an Agreement Reason of NEW_REQ")

@validates_schema
def validate_agreement_reason_must_have_incumbent(self, data, **kwargs):
def validate_agreement_reason_must_have_vendor(self, data, **kwargs):
if self.status_is_changing_beyond_draft(data):
bli = self.get_current_budget_line_item()
if (
Expand All @@ -150,10 +148,10 @@ def validate_agreement_reason_must_have_incumbent(self, data, **kwargs):
bli.agreement.agreement_reason == AgreementReason.RECOMPETE
or bli.agreement.agreement_reason == AgreementReason.LOGICAL_FOLLOW_ON
)
and not bli.agreement.incumbent_id
and not bli.agreement.vendor_id
):
raise ValidationError(
"BLI's Agreement must have an Incumbent if it has an Agreement Reason of RECOMPETE or LOGICAL_FOLLOW_ON"
"BLI's Agreement must have a Vendor if it has an Agreement Reason of RECOMPETE or LOGICAL_FOLLOW_ON"
)

@validates_schema
Expand Down
20 changes: 6 additions & 14 deletions backend/ops_api/tests/ops/agreement/test_agreement.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_agreements_get_all(auth_client, loaded_db, test_project):
assert response.json[0]["project"]["id"] == test_project.id
assert numpy.isclose(response.json[0]["budget_line_items"][0]["amount"], 1000000.0)
assert numpy.isclose(response.json[0]["procurement_shop"]["fee"], 0.0)
assert response.json[0]["incumbent"] == "Vendor 1"
assert response.json[0]["vendor"] == "Vendor 1"
assert "budget_line_items" in response.json[0]
assert "can_id" in response.json[0]["budget_line_items"][0]
assert "can" in response.json[0]["budget_line_items"][0]
Expand Down Expand Up @@ -88,7 +88,6 @@ def test_agreements_serialization(auth_client, loaded_db):
assert response.json["support_contacts"] == agreement.support_contacts
assert len(response.json["team_members"]) == len(agreement.team_members)
assert response.json["vendor_id"] == agreement.vendor_id
assert response.json["incumbent_id"] == agreement.incumbent_id
assert response.json["vendor"] == agreement.vendor.name


Expand Down Expand Up @@ -248,7 +247,6 @@ def test_contract(loaded_db, test_vendor, test_admin_user, test_project):
project_id=test_project.id,
created_by=test_admin_user.id,
vendor_id=test_vendor.id,
incumbent_id=test_vendor.id,
)

loaded_db.add(contract_agreement)
Expand Down Expand Up @@ -538,7 +536,6 @@ def test_agreements_post_contract_with_service_requirement_type(auth_client, loa
"name": "FRANK TEST",
"description": "test description",
"product_service_code_id": 1,
"incumbent": None,
"project_officer_id": 500,
"team_members": [
{
Expand All @@ -552,6 +549,7 @@ def test_agreements_post_contract_with_service_requirement_type(auth_client, loa
"awarding_entity_id": 2,
"contract_type": "FIRM_FIXED_PRICE",
"service_requirement_type": "SEVERABLE",
"vendor": None,
},
)
assert response.status_code == 201
Expand All @@ -562,7 +560,7 @@ def test_agreements_post_contract_with_service_requirement_type(auth_client, loa


@pytest.mark.usefixtures("app_ctx")
def test_agreements_post_contract_with_incumbent(auth_client, loaded_db, test_user, test_project):
def test_agreements_post_contract_with_vendor(auth_client, loaded_db, test_user, test_project):
response = auth_client.post(
"/api/v1/agreements/",
json={
Expand All @@ -571,7 +569,7 @@ def test_agreements_post_contract_with_incumbent(auth_client, loaded_db, test_us
"name": "REED TEST CONTRACT",
"description": "test description",
"product_service_code_id": 1,
"incumbent": "Vendor 1",
"vendor": "Vendor 1",
"project_officer_id": test_user.id,
"team_members": [
{
Expand Down Expand Up @@ -608,8 +606,6 @@ def test_agreements_patch_by_id_e2e(auth_client, loaded_db, test_contract, test_
"delivered_status": False,
"description": "Test Description",
"display_name": "Test Contract",
"incumbent": None,
"incumbent_id": None,
"name": "Test Edit Title",
"notes": "Test Notes test edit notes",
"po_number": None,
Expand Down Expand Up @@ -683,7 +679,7 @@ def test_agreements_patch_contract_by_id(auth_client, loaded_db, test_contract):
def test_agreements_patch_contract_update_existing_vendor(auth_client, loaded_db, test_contract):
response = auth_client.patch(
url_for("api.agreements-item", id=test_contract.id),
json={"vendor": "Vendor 2", "incumbent": "Vendor 2"},
json={"vendor": "Vendor 2"},
)
assert response.status_code == 200

Expand All @@ -703,15 +699,13 @@ def test_agreements_patch_contract_update_existing_vendor(auth_client, loaded_db
assert data["created_by"] is test_contract.created_by
assert data["vendor_id"] == 101
assert data["vendor"] == "Vendor 2"
assert data["incumbent_id"] == 101
assert data["incumbent"] == "Vendor 2"


@pytest.mark.usefixtures("app_ctx")
def test_agreements_patch_contract_update_new_vendor(auth_client, loaded_db, test_contract):
response = auth_client.patch(
url_for("api.agreements-item", id=test_contract.id),
json={"vendor": "Random Test Vendor", "incumbent": "Random Test Vendor"},
json={"vendor": "Random Test Vendor"},
)
assert response.status_code == 200

Expand All @@ -730,5 +724,3 @@ def test_agreements_patch_contract_update_new_vendor(auth_client, loaded_db, tes
assert data["created_by"] is test_contract.created_by
assert data["vendor_id"] == 103
assert data["vendor"] == "Random Test Vendor"
assert data["incumbent_id"] == 103
assert data["incumbent"] == "Random Test Vendor"
4 changes: 2 additions & 2 deletions backend/ops_api/tests/ops/agreement/test_agreement_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_agreement_history(auth_client, loaded_db, test_can):
"name": "Agreement144",
"description": "Description",
"product_service_code_id": 1,
"incumbent": "Vendor A",
"vendor": "Vendor A",
"project_officer_id": 500,
"team_members": [
{
Expand Down Expand Up @@ -123,7 +123,7 @@ def test_agreement_history_log_items(auth_client, app, test_can, utc_today):
"name": "TEST: Agreement history with change requests",
"description": "Description",
"product_service_code_id": 1,
"incumbent": "Vendor A",
"vendor": "Vendor A",
"project_officer_id": 520,
"team_members": [
{
Expand Down
Loading

0 comments on commit e33adc5

Please sign in to comment.