[Issue #3874] Create Rough Tables for Opportunity Applicate package #3906

babebe · 2025-02-14T21:19:31Z

Summary

Fixes #{3874}

Time to review: 15 mins

Rough Tables Created
opportunity_application_form
opportunity_competition_form
opportunity_competition
opportunity_competition_assistance_listing
opportunity_competition_instruction

Factories created

… 3875/opp-package-app-tables

chouinar · 2025-02-14T21:27:16Z

api/src/db/models/opportunity_models.py

@@ -451,3 +457,80 @@ class OpportunityChangeAudit(ApiSchemaTable, TimestampMixin):
        BigInteger, ForeignKey(Opportunity.opportunity_id), primary_key=True, index=True
    )
    opportunity: Mapped[Opportunity] = relationship(Opportunity)
+
+
+class OpportunityCompetition(ApiSchemaTable, TimestampMixin):


Could we put these in a competition_models.py file - just want to avoid the opportunity file getting really long, and there are going to be a lot of additional tables we need for this. We can also name the tables without prefixing with Opportunity - they're all sorta a part of it, but separate enough

chouinar · 2025-02-14T21:30:27Z

api/src/db/models/opportunity_models.py

+    opportunity_competition: Mapped[list["OpportunityCompetition"]] = relationship(
+        back_populates="opportunity", uselist=True, cascade="all, delete-orphan"
+    )


FYI - if you move these models to another file, you might hit an issue with this having a circular import. You can work around that by adding the import to the if TYPE_CHECKING line towards the top of this file: https://github.com/HHS/simpler-grants-gov/blob/main/api/src/db/models/opportunity_models.py#L26

chouinar · 2025-02-14T21:31:42Z

api/src/db/models/opportunity_models.py

+    __tablename__ = "opportunity_competition_assistance_listing"
+
+    competition_id: Mapped[uuid.UUID] = mapped_column(
+        UUID, ForeignKey(OpportunityCompetition.competition_id), index=True, primary_key=True


We don't need to index if it's the primary key - I might have something like that on some of our many-to-many tables, but that was a holdover from me testing some performance ideas a while back (I don't think they ever worked?)

Yes, makes sense. Removed

chouinar · 2025-02-14T21:32:41Z

api/tests/src/db/models/factories.py

+    opportunity = factory.SubFactory(OpportunityFactory)
+    opportunity_id = factory.LazyAttribute(lambda o: o.opportunity.opportunity_id)
+
+    competition_id = Generators.UuidObj


Could we put the primary key at the top of the factories? I generally just keep everything in the same order as the class itself.

Moved to top

chouinar · 2025-02-14T21:37:04Z

api/tests/src/db/models/factories.py

+    legacy_competition_id = sometimes_none(factory.Faker("random_int", min=1, max=15))
+    public_competition_id = sometimes_none(factory.Faker("random_int", min=1, max=15))
+    legacy_package_id = sometimes_none(factory.Faker("random_int", min=1, max=15))


For these, my recommendation:

don't set legacy_competition_id -> it's a pointer back to the old system, only our transform logic will ever care about it / want it set.

public_competition_id -> It's actually a string with some variance in how it can be set, I'd just put something like ABC<number> to make it look sorta like the old system (keep the sometimes_none)

legacy_package_id - This looks to be formatted as PKG<number>, probably just do that as well? (keep the sometimes_none).

chouinar · 2025-02-14T21:38:37Z

api/tests/src/db/models/factories.py

+    )
+
+    grace_period = factory.Faker("random_int", min=1, max=10)
+    contact_info = sometimes_none(factory.Faker("sentence"))


I setup a contact info faker provider previously, could use agency_contact_description for this to get it to roughly look like contact info

chouinar · 2025-02-14T21:43:52Z

api/tests/src/db/models/factories.py

+    form_id = Generators.UuidObj
+    form_name = factory.Faker("sentence")
+    form_version = factory.Faker("sentence")
+    is_active = False
+    description = factory.Faker("sentence")
+    agency_code_id = factory.Faker("sentence")


Some suggestions:

form_name -> bs might be a good one: https://faker.readthedocs.io/en/master/providers/faker.providers.company.html#faker.providers.company.Provider.bs

form_version -> I'd probably say we should make this formatted like 1.<random int> - could randomize both, don't think it matters much

is_active -> default to true, if it's inactive, we'll probably disallow the form for use in certain ways

agency_code_id -> Just reuse the agency_code faker provider we made

chouinar · 2025-02-14T21:44:23Z

api/src/db/models/opportunity_models.py

+    form_version: Mapped[str]
+    is_active: Mapped[bool]
+    description: Mapped[str]
+    agency_code_id: Mapped[str]


On second thought, let's just name this agency_code like we have on the opportunity

chouinar · 2025-02-14T21:48:54Z

api/src/db/models/opportunity_models.py

+    form_name: Mapped[str]
+    form_version: Mapped[str]
+    is_active: Mapped[bool]
+    description: Mapped[str]


Yknow what, on second thought, I don't even know what this description refers to, let's drop this column. It's not on the form UI, so let's hold off on it for now

…simpler-grants-gov into 3875/opp-package-app-tables

… 3875/opp-package-app-tables

…simpler-grants-gov into 3875/opp-package-app-tables

mdragon · 2025-02-18T15:59:53Z

api/src/db/migrations/versions/2025_02_14_add_competition_tables.py

+        sa.Column("form_id", sa.UUID(), nullable=False),
+        sa.Column("form_name", sa.Text(), nullable=False),
+        sa.Column("form_version", sa.Text(), nullable=False),
+        sa.Column("is_active", sa.Boolean(), nullable=False),


We should think through the lifecycle a bit more than just currently active or inactive. Do we need to have a "available after"/"available until" kind of dates when a form is "depreciated" or "retired?" For example what if SF-424 v4 is live now and v5 gets approved and added to the system. Do all applications switch immediately? Do new application packages only show v5 when selecting what forms to add? Is there a period where both remain pick-able when setting up an application package? Even if we just have active, I think it'd be good to have this be a date/time and have it be called like inactive_at and that date then controls if it's shown and also provides more of a history of when it became inactivated, can be set for a future date, etc.

If we want to move this forward in the short term, maybe just make this a timestamp instead of boolean and re-name it accordingly and then we can always add an "active_at" column in the future if we need more specifics of a lifecycle.

@mdragon These were initially considered basic columns, with the plan to add more as needed. I believe it's a good idea to include both inactive_at and active_at columns, as it's generally better for each column to serve a single purpose rather than combining logic. Thoughts?

I think that makes sense for now and then yes, we absolutely shouldn't try to make perfect tables out of the gate and anticipate revisions and improvements as we work with the tables and learn more about the functionality. 👍🏻

Added inactive_at -nullable and active_at-non-nullable datetime columns and updated factory

… 3875/opp-package-app-tables

babebe added 27 commits February 3, 2025 09:25

add route

0fc094a

input/output schema

c414537

function to update

c3064e1

add test

89a4b8f

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov

e281670

merge main

f73d28a

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov

4d0a32b

merge

5a3d03a

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov

fb56555

merge main

147a51d

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov

45a33b9

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov

714961d

del service func

ca1acee

del schema

e314b9b

del test

2fb586b

merge main

b107943

model, fact , migration file for competition

3690bbd

fmt

3b9eb31

add OpportunityCompetitionInstruction model, factory, migration

51d3fe2

change names

3987e4f

update type

00ea753

recreaate migration

1cd9788

add m2m table

9f715f0

add ApplicationForm

3338455

add OpportunityCompetitionForm

82c616f

all tables migration file

730c240

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov into…

1355f3f

… 3875/opp-package-app-tables

babebe requested review from chouinar and mdragon as code owners February 14, 2025 21:19

Create ERD diagram and Update OpenAPI spec

ba382d6

mv tbls to competition_models

61d6804

chouinar reviewed Feb 14, 2025

View reviewed changes

babebe and others added 10 commits February 14, 2025 16:59

cleanup names

821f80a

Merge branch '3875/opp-package-app-tables' of https://github.com/HHS/…

8bbabba

…simpler-grants-gov into 3875/opp-package-app-tables

Create ERD diagram and Update OpenAPI spec

4bff347

cleanup

2c3587e

add CompetitionAssistanceListingFactory

b5098c1

revised migration

d941982

Merge branch '3875/opp-package-app-tables' of https://github.com/HHS/…

c806562

…simpler-grants-gov into 3875/opp-package-app-tables

Create ERD diagram and Update OpenAPI spec

b879f11

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov into…

f09d9c1

… 3875/opp-package-app-tables

Merge branch '3875/opp-package-app-tables' of https://github.com/HHS/…

44dcd96

…simpler-grants-gov into 3875/opp-package-app-tables

babebe requested a review from mikehgrantsgov February 18, 2025 16:27

mdragon requested changes Feb 18, 2025

View reviewed changes

babebe requested a review from mdragon February 18, 2025 18:26

babebe and others added 5 commits February 18, 2025 16:45

add active_at, inactive_at

ab2f750

update factory

48d3890

update factory

a85cfb9

Merge branch 'main' of https://github.com/HHS/simpler-grants-gov into…

d5352b5

… 3875/opp-package-app-tables

Create ERD diagram and Update OpenAPI spec

5dc8eea

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue #3874] Create Rough Tables for Opportunity Applicate package #3906

[Issue #3874] Create Rough Tables for Opportunity Applicate package #3906

babebe commented Feb 14, 2025

chouinar Feb 14, 2025

chouinar Feb 14, 2025

babebe Feb 14, 2025

chouinar Feb 14, 2025

babebe Feb 14, 2025

chouinar Feb 14, 2025

babebe Feb 14, 2025

chouinar Feb 14, 2025

chouinar Feb 14, 2025

chouinar Feb 14, 2025

chouinar Feb 14, 2025

chouinar Feb 14, 2025

mdragon Feb 18, 2025

babebe Feb 18, 2025 •

edited

Loading

mdragon Feb 18, 2025

babebe Feb 18, 2025 •

edited

Loading

[Issue #3874] Create Rough Tables for Opportunity Applicate package #3906

Are you sure you want to change the base?

[Issue #3874] Create Rough Tables for Opportunity Applicate package #3906

Conversation

babebe commented Feb 14, 2025

Summary

Time to review: 15 mins

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

babebe Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

babebe Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

babebe Feb 18, 2025 •

edited

Loading

babebe Feb 18, 2025 •

edited

Loading