From 86d23da207bc55bc3150f396b641a3be7e3a36b3 Mon Sep 17 00:00:00 2001 From: Brandon Tabaska Date: Wed, 1 May 2024 16:58:25 -0400 Subject: [PATCH 1/2] [Issue #1531] Setup env for App Router In next.js GA4 implementation (#1896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #{1531} ### Time to review: __2 mins__ ## Changes proposed > Added GOOGLE_ANALYTICS_ID to frontend public ENVs on production to resolve issues with GA4 on app router. > Renamed GOOGLE_TAG_ID to GOOGLE_TAG_MANAGER_ID to have the variable named properly ## Context for reviewers > GA4 requires a GOOGLE_ANALYTICS_ID in addition to a GOOGLE_TAG_MANAGER_ID to handoff tracking between app router and page router in next.js ## Additional information > ![Screenshot 2024-05-01 at 12 41 22 PM](https://github.com/HHS/simpler-grants-gov/assets/29316916/a07b4b6e-a6d3-404b-945a-e53da1042e03) --- frontend/src/app/layout.tsx | 2 +- frontend/src/constants/environments.ts | 9 ++++++--- frontend/src/pages/_app.tsx | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/layout.tsx b/frontend/src/app/layout.tsx index bf3352c40..707c3ad10 100644 --- a/frontend/src/app/layout.tsx +++ b/frontend/src/app/layout.tsx @@ -33,7 +33,7 @@ export default function RootLayout({ children }: LayoutProps) { {children} {process.env.NEXT_PUBLIC_ENVIRONMENT === "prod" && ( - + )} ); diff --git a/frontend/src/constants/environments.ts b/frontend/src/constants/environments.ts index aea740956..01ab36724 100644 --- a/frontend/src/constants/environments.ts +++ b/frontend/src/constants/environments.ts @@ -4,13 +4,16 @@ */ const PUBLIC_ENV_VARS_BY_ENV = { development: { - GOOGLE_TAG_ID: "GTM-MV57HMHS", + GOOGLE_TAG_MANAGER_ID: "GTM-MV57HMHS", + GOOGLE_ANALYTICS_ID: "G-6MDCC5EZW2", }, test: { - GOOGLE_TAG_ID: "GTM-MV57HMHS", + GOOGLE_TAG_MANAGER_ID: "GTM-MV57HMHS", + GOOGLE_ANALYTICS_ID: "G-6MDCC5EZW2", }, production: { - GOOGLE_TAG_ID: "GTM-MV57HMHS", + GOOGLE_TAG_MANAGER_ID: "GTM-MV57HMHS", + GOOGLE_ANALYTICS_ID: "G-6MDCC5EZW2", }, } as const; diff --git a/frontend/src/pages/_app.tsx b/frontend/src/pages/_app.tsx index 10db979da..5cf80b206 100644 --- a/frontend/src/pages/_app.tsx +++ b/frontend/src/pages/_app.tsx @@ -20,7 +20,7 @@ function MyApp({ Component, pageProps }: AppProps) { {process.env.NEXT_PUBLIC_ENVIRONMENT === "prod" && ( - + )} From b4b9bf5a488cf1967fc16fc413fff42c738bdd9a Mon Sep 17 00:00:00 2001 From: Michael Chouinard <46358556+chouinar@users.noreply.github.com> Date: Thu, 2 May 2024 11:13:07 -0400 Subject: [PATCH 2/2] [Issue #1905] Add unique constraint for opportunity summary (#1906) ## Summary Fixes #1905 ### Time to review: __5 mins__ ## Changes proposed Upgrade our local version of Postgres to version 15 Add a unique constraint to the opportunity summary for ## Context for reviewers This constraint is something that is currently implicitly handled in the legacy system across 4 tables, but we're merging into one table. Adding this constraint makes us more certain that we're not importing data in the transformation process incorrectly. - `is_forecast` - whether it came from the forecast or synopsis table (or their history equivalents) - `revision_number` - null from the non-historical tables, otherwise a part of the primary key - `opportunity_id` - part of the primary key in all of the source Oracle tables Together they let us uniquely point back to a record in the legacy system without issue. Note that because revision number can be null, to make it work correctly in a unique constraint we need to use the `NULLS NOT DISTINCT` flag when creating the unique index: https://www.postgresql.org/docs/current/indexes-unique.html - this was only added in Postgres 15. While there are some other workarounds in older versions, we're running Postgres 15 in AWS, so I see zero reason not to upgrade and match. What this flag does is basically make it so in a compound unique key, null is treated as a distinct value. For example, lets say we have values of `True, null, 1`. Without `NULLS NOT DISTINCT` you could keep inserting this same 3 values because it would think null != null. ## Additional information Note that you will need to run `make db-recreate` as the volume that backs a v14 and v15 Postgres DB are incompatible. This will drop your DB and remake it. --- api/docker-compose.yml | 2 +- ...05_02_add_unique_constraint_for_summary.py | 37 +++++++++++++++++++ api/src/db/models/base.py | 2 +- api/src/db/models/opportunity_models.py | 12 +++++- 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 api/src/db/migrations/versions/2024_05_02_add_unique_constraint_for_summary.py diff --git a/api/docker-compose.yml b/api/docker-compose.yml index 44cb64454..a364c74c3 100644 --- a/api/docker-compose.yml +++ b/api/docker-compose.yml @@ -3,7 +3,7 @@ version: '3' services: grants-db: - image: postgres:14-alpine + image: postgres:15-alpine container_name: grants-db command: postgres -c "log_lock_waits=on" -N 1000 -c "fsync=off" env_file: ./local.env diff --git a/api/src/db/migrations/versions/2024_05_02_add_unique_constraint_for_summary.py b/api/src/db/migrations/versions/2024_05_02_add_unique_constraint_for_summary.py new file mode 100644 index 000000000..8ed298dfb --- /dev/null +++ b/api/src/db/migrations/versions/2024_05_02_add_unique_constraint_for_summary.py @@ -0,0 +1,37 @@ +"""add unique constraint for summary + +Revision ID: 24061ff82646 +Revises: 1ddd1d051a99 +Create Date: 2024-05-02 10:11:35.832837 + +""" +from alembic import op + +# revision identifiers, used by Alembic. +revision = "24061ff82646" +down_revision = "1ddd1d051a99" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint( + op.f("opportunity_summary_is_forecast_uniq"), + "opportunity_summary", + ["is_forecast", "revision_number", "opportunity_id"], + schema="api", + postgresql_nulls_not_distinct=True, + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint( + op.f("opportunity_summary_is_forecast_uniq"), + "opportunity_summary", + schema="api", + type_="unique", + ) + # ### end Alembic commands ### diff --git a/api/src/db/models/base.py b/api/src/db/models/base.py index ee739fb91..3844a1ad6 100644 --- a/api/src/db/models/base.py +++ b/api/src/db/models/base.py @@ -100,7 +100,7 @@ def __rich_repr__(self) -> Iterable[tuple[str, Any]]: class ApiSchemaTable(Base): __abstract__ = True - __table_args__ = {"schema": Schemas.API} + __table_args__: Any = {"schema": Schemas.API} @declarative_mixin diff --git a/api/src/db/models/opportunity_models.py b/api/src/db/models/opportunity_models.py index 9f19dcd4f..2f8287298 100644 --- a/api/src/db/models/opportunity_models.py +++ b/api/src/db/models/opportunity_models.py @@ -1,6 +1,6 @@ from datetime import date -from sqlalchemy import BigInteger, ForeignKey +from sqlalchemy import BigInteger, ForeignKey, UniqueConstraint from sqlalchemy.ext.associationproxy import AssociationProxy, association_proxy from sqlalchemy.orm import Mapped, mapped_column, relationship @@ -86,6 +86,16 @@ def opportunity_status(self) -> OpportunityStatus | None: class OpportunitySummary(ApiSchemaTable, TimestampMixin): __tablename__ = "opportunity_summary" + __table_args__ = ( + # nulls not distinct makes it so nulls work in the unique constraint + UniqueConstraint( + "is_forecast", "revision_number", "opportunity_id", postgresql_nulls_not_distinct=True + ), + # Need to define the table args like this to inherit whatever we set on the super table + # otherwise we end up overwriting things and Alembic remakes the whole table + ApiSchemaTable.__table_args__, + ) + opportunity_summary_id: Mapped[int] = mapped_column(BigInteger, primary_key=True) opportunity_id: Mapped[int] = mapped_column(