chore: remove links between question instances and (detail) responses #4141
Conversation
|
Test summaryRun details
Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud |
||||||||||||||||||||||||||
There was a problem hiding this comment.
Agree with the change and makes the schema cleaner and data consistent. However, to be discussed whether we want to delete element instances and thus trigger cascading delete (or whether we remove cascading delete and actually want to have a transaction fail when an instance is deleted but responses are still around?), or whether we keep element instances around even if the element they correspond to is deleted (or even also for quizzes that were published but are deleted?), thus keeping leaderboards and responses consistent (as the instance should contain all data necessary to know what it was about)? Especially if we want to do timelines, audit logs, and such things, we want to make sure that they cannot change after participants have seen them, and responses should be consistent with such derived data (meaning responses should basically never be deleted).
WalkthroughThis migration overhaul enhances the database schema by removing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Database
User->>App: Submit Question Response
App->>Database: Save Response with Element Instance
Database-->>App: Confirmation
App-->>User: Response Saved
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/prisma/src/prisma/migrations/20240715083055_drop_question_instance_response_links/migration.sql (1 hunks)
- packages/prisma/src/prisma/schema.prisma (3 hunks)
Additional comments not posted (11)
packages/prisma/src/prisma/migrations/20240715083055_drop_question_instance_response_links/migration.sql (9)
10-11: Approved: Dropping foreign key constraint.Dropping the foreign key constraint
QuestionResponse_elementInstanceId_fkeyis necessary for modifying theelementInstanceIdcolumn.
13-14: Approved: Dropping foreign key constraint.Dropping the foreign key constraint
QuestionResponse_questionInstanceId_fkeyis necessary as thequestionInstanceIdcolumn is being removed.
16-17: Approved: Dropping foreign key constraint.Dropping the foreign key constraint
QuestionResponseDetail_elementInstanceId_fkeyis necessary for modifying theelementInstanceIdcolumn.
19-20: Approved: Dropping foreign key constraint.Dropping the foreign key constraint
QuestionResponseDetail_questionInstanceId_fkeyis necessary as thequestionInstanceIdcolumn is being removed.
22-23: Approved: Dropping index.Dropping the index
QuestionResponse_participantId_questionInstanceId_keyis necessary as thequestionInstanceIdcolumn is being removed.
33-34: Approved: Adding foreign key constraint.Adding the foreign key constraint
QuestionResponse_elementInstanceId_fkeyensures referential integrity with theElementInstancetable.
36-37: Approved: Adding foreign key constraint.Adding the foreign key constraint
QuestionResponseDetail_elementInstanceId_fkeyensures referential integrity with theElementInstancetable.
29-31: Approved: AlteringQuestionResponseDetailtable.Dropping the
questionInstanceIdcolumn and setting theelementInstanceIdcolumn to NOT NULL is necessary for the schema changes.Ensure that there are no existing NULL values in the
elementInstanceIdcolumn before applying this change.
25-27: Approved: AlteringQuestionResponsetable.Dropping the
questionInstanceIdcolumn and setting theelementInstanceIdcolumn to NOT NULL is necessary for the schema changes.Ensure that there are no existing NULL values in the
elementInstanceIdcolumn before applying this change.packages/prisma/src/prisma/schema.prisma (2)
1015-1016: Approved: Changes toQuestionResponsemodel.The removal of the
questionInstanceIdfield and making theelementInstancerelationship mandatory with ON DELETE and ON UPDATE CASCADE is correctly reflected.
1042-1043: Approved: Changes toQuestionResponseDetailmodel.The removal of the
questionInstanceIdfield and making theelementInstancerelationship mandatory with ON DELETE and ON UPDATE CASCADE is correctly reflected.
|
Test summaryRun details
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud |
This pull request makes the following changes to the database scheme (incl. migration):
QuestionInstance(only used in live quiz) andQuestionResponseandQuestionResponseDetails- all corresponding ids are set to null alreadyQuestionResponse/QuestionResponseDetailandElementInstancerequired, since response objects without this link cannot be associated with a question / flashcard / ... anyway and are therefore not really informative - all of the corresponding ids are defined (not response withnullaselementInstanceId)ElementInstance, since there might be responses without an assigned question otherwise, which does not make sense, since the results cannot be interpreted without the question.Summary by CodeRabbit
New Features
elementInstanceIda required field in theQuestionResponseandQuestionResponseDetailtables.Bug Fixes
questionInstanceIdfield from theQuestionResponseandQuestionResponseDetailmodels to streamline relationships.Refactor
ElementInstance, enhancing the overall structure of the data model.