-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: master
Are you sure you want to change the base?
Conversation
dd6529b
to
a2ab828
Compare
343b2ed
to
5c4652f
Compare
There was a problem hiding this 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/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legacy ./prisma
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, thanks
b6f4702
to
275f902
Compare
275f902
to
ca6ff8d
Compare
There was a problem hiding this 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; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Setup