Navigation Menu

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

Skip AlterUniqueTogether for logger_xform #693

Merged
merged 1 commit into from Mar 18, 2021

Conversation

jnm
Copy link
Member

@jnm jnm commented Mar 17, 2021

…and rely instead on the unique_together constraint being
automatically removed when its constituent column is dropped

Tested successfully on:

  • kc.beta.kbtdev.org
  • locally in a new, empty environment

…and rely instead on the `unique_together` constraint being
automatically removed when its constituent column is dropped
@jnm jnm requested a review from noliveleger March 17, 2021 01:00
@jnm jnm changed the title [:stop_sign: untested!] Skip AlterUniqueTogether for logger_xform [🛑 untested!] Skip AlterUniqueTogether for logger_xform Mar 17, 2021
@jnm jnm changed the title [🛑 untested!] Skip AlterUniqueTogether for logger_xform Skip AlterUniqueTogether for logger_xform Mar 17, 2021
@jnm jnm marked this pull request as ready for review March 17, 2021 16:51
@jnm jnm removed their assignment Mar 17, 2021
Comment on lines -15 to -18
migrations.AlterUniqueTogether(
name='xform',
unique_together={('user', 'id_string')},
),
Copy link
Contributor

Choose a reason for hiding this comment

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

It does not restore the constraint if we want to rollback.

As per discussed:

Three ideas:

  • move AlterUniqueTogether after the column removal (seems like a long shot)
  • add a second migration for the AlterUniqueTogether
  • Create a reverse function which adds the contraint.

Copy link
Member Author

@jnm jnm Mar 17, 2021

Choose a reason for hiding this comment

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

Thanks for catching this. It's not great, but it seems like it's impossible to reverse this meaningfully because:

  • sms_id_string was not nullable and defaulted to an default empty string [sorry, typo];
  • when we drop the column, we don't save its values anywhere;
  • if we run the RemoveField backwards, it populates sms_id_string with an empty string for each logger_xform row;
  • attempting to add the unique_together constraint fails for any user who has more than one form. For example, consider user 123 has two forms; the constraint would see (123, '') twice and fail with:
    django.db.utils.IntegrityError: could not create unique index "logger_xform_user_id_sms_id_string_97791222_uniq"
    DETAIL:  Key (user_id, sms_id_string)=(123, ) is duplicated.
    

I guess we could fill in unique, dummy data in sms_id_string, but would that help anyone?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that it won't help. The SMS feature has been hidden/broken for a while on kobocat anyway. Thank you for the explanation.

@noliveleger noliveleger merged commit bb5492d into beta Mar 18, 2021
@noliveleger noliveleger deleted the skip-alteruniquetogether branch March 18, 2021 12:37
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.

None yet

2 participants