Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database schema / seed + other setup #1

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

jonnywyatt
Copy link
Contributor

@jonnywyatt jonnywyatt commented Apr 22, 2021

Setup

  • set up Prisma including data migration for initial table setup (ERD)
  • DB seed Channels / Countries
  • pre-commit hooks using Husky
  • readme
  • docker-compose for Postgres only (need to add container for Node API)

@jonnywyatt jonnywyatt changed the title Database setup & Subscriptions endpoints Database schema / seed + other setup Apr 26, 2021
@jonnywyatt jonnywyatt marked this pull request as ready for review April 26, 2021 14:34
Copy link

@bertrandmc bertrandmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment. LPGTM.

Dockerfile Outdated
RUN mkdir -p /usr/src/app
WORKDIR /usr/src/app
COPY package.json package-lock.json babel.config.json tsconfig.json ./
COPY ./prisma ./prisma/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legacy ./prisma?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks

@jonnywyatt jonnywyatt force-pushed the subscriptions branch 2 times, most recently from b6f4702 to 275f902 Compare April 27, 2021 12:54
Copy link
Collaborator

@alisalaman-cyb alisalaman-cyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jon, just some comments on the FKs from me.

CREATE UNIQUE INDEX "Subscription.recipientId_recipientIdPrefix_channelId_countryCode_unique" ON "Subscription"("recipientId", "recipientIdPrefix", "channelId", "countryCode");

-- AddForeignKey
ALTER TABLE "Alert" ADD FOREIGN KEY ("staffId") REFERENCES "Staff"("id") ON DELETE CASCADE ON UPDATE CASCADE;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to preserve audit history, would it be better to soft-delete / change the status of a staff member to "departed" or "inactive" rather than hard-deleting them from the database and cascading the delete down to the referenced tables? Same would apply for the other FKs using ON DELETE CASCADE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout, I'll do that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Prisma will add support for configuring cascading deletes via the schema, in the next release (early May) - prisma/prisma#2810
I'll hold this until then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants