Skip to content

Fix silent deserialization failure with unique together constraint#313

Merged
bjester merged 7 commits intolearningequality:release-v0.8.xfrom
ozer550:fix-silent-deserialization-failure-with-unique-together-constraint
Apr 9, 2026
Merged

Fix silent deserialization failure with unique together constraint#313
bjester merged 7 commits intolearningequality:release-v0.8.xfrom
ozer550:fix-silent-deserialization-failure-with-unique-together-constraint

Conversation

@ozer550
Copy link
Copy Markdown
Member

@ozer550 ozer550 commented Apr 2, 2026

Summary

  • Added IntegrityError to the caught exceptions in the deserialization pipeline so unique constraint violations are handled instead of silently ignored.
  • Replaced the bulk upsert in the non-self-referential branch with per-record saves.
  • Added regression tests in test_controller.py for the unique constraint scenario.

TODO

  • Have tests been written for the new code?
  • Has documentation been written/updated?
  • New dependencies (if any) added to requirements file

Reviewer guidance

  • Check if the regression tests make sense!
  • Manually syncing two devices with voilating unique constraint for fields.

Issues addressed

closes #306

:returns: True if save succeeded, False otherwise
"""
try:
with transaction.atomic():
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is so that the outer transaction stays healthy, and we can keep saving other records.

Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

This is really mostly complete, just some cleanup and tidying code.

And the small deserialization_exception enhancement!

@ozer550 ozer550 marked this pull request as ready for review April 6, 2026 15:57
@ozer550 ozer550 changed the base branch from release-v0.9.x to release-v0.8.x April 6, 2026 15:59
@bjester bjester self-assigned this Apr 7, 2026
Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

Looks great! The TDD driven approach worked really well here. Thanks for tackling this @ozer550

@bjester bjester merged commit 5a9e53d into learningequality:release-v0.8.x Apr 9, 2026
24 checks passed
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.

2 participants