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

Communication: Reintroduce foreign key constraints and offline primary keys #70

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

PaRangger
Copy link
Collaborator

@PaRangger PaRangger commented Oct 24, 2024

Motivation

In the hotfix for version 1.0.1 and 1.0.2 the foreign key constraints for reactions were removed. Additionally, the primary key constraints for posts and reactions were altered to include the server_id. These changes were done to prevent the application from crashing. This however introduced the problem that while the app was running without crashes, the data integrity is not given anymore.

Description

Pull Request #58 fixed an issue with the answer threads not being shown. This issue also seemed to be the origin of the failing primary and foreign key constraints. Therefore I reverted the changes to include the previous FK and PK checks as they were.

Closes #62

Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

Tested the App and works as expected without any crashes.

Also tested the migration from the 1.0.3 release to this app state, everything works fine 👍

@PaRangger PaRangger marked this pull request as ready for review October 24, 2024 14:38
Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

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

Just tested it, and everything seems to work fine.

Copy link
Contributor

@TimOrtel TimOrtel left a comment

Choose a reason for hiding this comment

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

Please add some unit Tests to verify that the referential integrity works by writing tests that add messages, delete messages, delete conversations etc.

@FelberMartin FelberMartin self-assigned this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Communication: Index constraints cause crashes
5 participants