Conversation
jasnoo
left a comment
There was a problem hiding this comment.
I haven't manually tested anything yet so I'm not done with this review, but I just wanted to make my comments visible for others to chime in on
sannidhishukla
left a comment
There was a problem hiding this comment.
looks good! just added a few thoughts
AnJuHyppolite
left a comment
There was a problem hiding this comment.
Thank you for this work, Lizzy! I left a few comments about the PR cover.
|
|
||
| As a workaround, the init migration (prisma/migrations/20250623215747_init/ migration.sql) has been manually customized to set the text_pattern_ops operator. | ||
| **/ | ||
| @@index([pageUrl, createdAt]) |
There was a problem hiding this comment.
the new index! adding on a little more context beyond the code comment:
- i chose to list
pageUrlcolumn first in the index because I assume the most common queries will involve filtering based onpageUrlfirst andcreatedAtsecond- note that this index will not make queries that filter solely based on
createdAtmore efficient (see PostgreSQL docs on multicolumn indices)
- note that this index will not make queries that filter solely based on
There was a problem hiding this comment.
updated the README to describe how to manage migrations!
README.md
Outdated
| - In practice, do this by deleting the migrations you want to squash from the `migrations/` directory | ||
| 2. Reset the local database by running `npx prisma migrate reset` (or `npm run prisma-migrate:reset`) | ||
| 3. Run `npx prisma migrate dev` | ||
| - When we do this, Prisma will apply all the migrations in the `migrations` folder in order to the fresh database. |
There was a problem hiding this comment.
[pebble] Should this say refresh the database?
There was a problem hiding this comment.
I meant fresh database in the sense the database is empty since we just ran npx prisma migrate reset. edited clarity it for clarity to:
When we do this, Prisma will sequentially apply all the migrations in the
migrationsfolder to the freshly reset database.
Let me know if this makes sense or if there's a better way to reword it!
<!-- Please complete the following sections as necessary. --> ### Description Creates the `Feedback` Prisma data model, as well as the initial migration to load this schema into our postgres database. ### Ticket Resolves [Ticket #430](newjersey/innovation-platform-pm#430) (Define a basic schema for the feedback database using Prisma/Prisma Migrate) ### Approach - Added the `@prisma/client` package - Added `DATABASE_URL` to .env - This is set to the local db URL (`postgresql://postgres:postgres@localhost:5432/postgres`) - I've also updated the Bitwarden with the new .env - I changed the database server time zone from `UTC` to `US/Eastern` by editing the `TZ` env var in the `docker-compose.yml` - We originally chose UTC because we didn't want to manage between switching from EDT/EST, but the`US/Eastern` time zone automatically handles this for us. - With the current database schema, when we create a new feedback record, the `createdAt` column defaults to the time the record was created. By setting the database time zone to `US/Eastern`, it will automatically generate the timestamp in Eastern time. - So when we're looking up records in the database, the dates will be more intuitive since they will match NJ's time zone, which is generally the timezone that people are submitting feedback from. - This might also make integrating with Looker easier as we don't have to convert the time from another timezone when displaying dates. ### Steps to Test #### Loading the schema into the local db If this is your first time running the local db, follow the [instructions in the README](https://github.com/newjersey/feedback-api/blob/dev/README.md#running-the-local-dev-database) first. 1. Run `npm install` 2. Delete the `feedback-api_postgres-data` volume since we don't want old persisted data from the local db - Run `docker volume ls` to see a list of volumes - If the volume is present, run `docker volume rm feedback-api_postgres-data` to delete it 3. Start the local db by running `colima start` then `docker compose up -d` 4. Run `npx prisma migrate dev` to apply the migrations in `prisma/migrations/` to the local db 5. Generate the Prisma Client containing our database schema's data models with `npx prisma generate` #### Manual testing strategy I didn't want to open the integration testing can of works yet, I decided to manually test that inserting into the Feedback table with Prisma works using a ts script, specifically paying attention that the `createdAt` column is correct. 1. Installed ts-node without saving it to package.json by running `npm install ts-node --no-save` 2. Created a `test.ts` file in the `prisma/` directory with the following code: ```ts import { PrismaClient } from '..//generated/prisma/client'; const prisma = new PrismaClient(); async function main() { await prisma.feedback.deleteMany({}); const feedbackWithoutTime = await prisma.feedback.create({ data: { rating: true, pageUrl: 'testUrl', createdAtFromApi: new Date() } }); const feedbackWithTime = await prisma.feedback.create({ data: { rating: true, pageUrl: 'testUrl', createdAt: new Date(1671717203269), // Thursday, December 22, 2022 1:53:23.269 PM UTC createdAtFromApi: new Date() } }); console.log({ feedbackWithoutTime, feedbackWithTime }); } main() .then(async () => { await prisma.$disconnect(); }) .catch(async (e) => { console.error(e); await prisma.$disconnect(); process.exit(1); }); ``` 3. Ran the script with `npx ts-node prisma/test.ts`. This produced the following console output: ```bash { feedbackWithoutTime: { id: 1, createdAt: 2025-07-22T19:43:28.141Z, createdAtFromApi: 2025-07-22T19:43:28.139Z, rating: true, pageUrl: 'testUrl', comment: null, email: null }, feedbackWithTime: { id: 2, createdAt: 2022-12-22T13:53:23.269Z, createdAtFromApi: 2025-07-22T19:43:28.147Z, rating: true, pageUrl: 'testUrl', comment: null, email: null } } ``` - Success criteria - The ids are being generated sequentially - For `feedbackWithoutTime`, the createdAt matches the time at which I ran the script (formatted in UTC) - For `feedbackWith`, the createdAt matches the inputted DateTime object (formatted in UTC) 4. Then, checked that the objects were created as expected in the database itself. Ran `psql postgresql://postgres:postgres@localhost:5432/postgres` to start the psql console. 5. Within the psql console, ran `SELECT * FROM feedback;`. This gave the following output ``` id | created_at | created_at_from_api | rating | page_url | comment | email ----+----------------------------+----------------------------+--------+----------+---------+------- 1 | 2025-07-22 15:43:28.141-04 | 2025-07-22 15:43:28.139-04 | t | testUrl | | 2 | 2022-12-22 08:53:23.269-05 | 2025-07-22 15:43:28.147-04 | t | testUrl | | (2 rows) ``` - Success criteria - The `id`, `rating`, and `pageUrl` fields match what we inserted with prisma. - The `createdAt` and `createdAtFromApi` fields are displayed in "US/Eastern" time, i.e. it automatically selects the appropriate UTC offset depending on whether DST was in effect on the given date. - The Prisma camelCase fields (e.g. `createdAt`) are mapped to snake_case based on the `@map` attributes in prisma.schema.
Description
Creates the
FeedbackPrisma data model, as well as the initial migration to load this schema into our postgres database.Ticket
Resolves Ticket #430 (Define a basic schema for the feedback database using Prisma/Prisma Migrate)
Approach
@prisma/clientpackageDATABASE_URLto .envpostgresql://postgres:postgres@localhost:5432/postgres)UTCtoUS/Easternby editing theTZenv var in thedocker-compose.ymlUS/Easterntime zone automatically handles this for us.createdAtcolumn defaults to the time the record was created. By setting the database time zone toUS/Eastern, it will automatically generate the timestamp in Eastern time.Steps to Test
Loading the schema into the local db
If this is your first time running the local db, follow the instructions in the README first.
npm installfeedback-api_postgres-datavolume since we don't want old persisted data from the local dbdocker volume lsto see a list of volumesdocker volume rm feedback-api_postgres-datato delete itcolima startthendocker compose up -dnpx prisma migrate devto apply the migrations inprisma/migrations/to the local dbnpx prisma generateManual testing strategy
I didn't want to open the integration testing can of works yet, I decided to manually test that inserting into the Feedback table with Prisma works using a ts script, specifically paying attention that the
createdAtcolumn is correct.npm install ts-node --no-savetest.tsfile in theprisma/directory with the following code:npx ts-node prisma/test.ts. This produced the following console output:{ feedbackWithoutTime: { id: 1, createdAt: 2025-07-22T19:43:28.141Z, createdAtFromApi: 2025-07-22T19:43:28.139Z, rating: true, pageUrl: 'testUrl', comment: null, email: null }, feedbackWithTime: { id: 2, createdAt: 2022-12-22T13:53:23.269Z, createdAtFromApi: 2025-07-22T19:43:28.147Z, rating: true, pageUrl: 'testUrl', comment: null, email: null } }feedbackWithoutTime, the createdAt matches the time at which I ran the script (formatted in UTC)feedbackWith, the createdAt matches the inputted DateTime object (formatted in UTC)psql postgresql://postgres:postgres@localhost:5432/postgresto start the psql console.SELECT * FROM feedback;. This gave the following outputid,rating, andpageUrlfields match what we inserted with prisma.createdAtandcreatedAtFromApifields are displayed in "US/Eastern" time, i.e. it automatically selects the appropriate UTC offset depending on whether DST was in effect on the given date.createdAt) are mapped to snake_case based on the@mapattributes in prisma.schema.