zerver: Replace log_event with RealmAuditLog in do_change_avatar_fields #4278
Conversation
|
Automated message from Dropbox CLA bot @misteraverin, it looks like you've already signed the Dropbox CLA. Thanks! |
|
@misteraverin This looks pretty good. Can you squash this into one commit, rebase against a current copy of master, and re-run the migrations again in your dev environment to ensure that no other migrations have hit master since your original change? |
|
@showell I merge migrations, but an error still appears |
|
@misteraverin Thanks for the progress update! Do you need help understanding the Travis output or debugging zerver.tests.test_audit_log.TestChangeAvatarFields.test_change_avatar_source? If so, maybe find somebody on chat (including me) who can help you. |
|
@showell Well, actually problem is "value too long for type character varying(1)", when I add new field to model |
zerver/tests/test_audit_log.py
Outdated
| realm = get_realm('zulip') | ||
| now = timezone.now() | ||
| user = do_create_user('email', 'password', realm, 'full_name', 'short_name') | ||
| avatar_source = '/img/1.jpg' |
There was a problem hiding this comment.
Your bug is because valid values for avatar_source are things like AVATAR_FROM_GRAVATAR = G. Read the zerver/models.py code for this field and you'll understand.
There was a problem hiding this comment.
yeah, great thanks
| @@ -1552,3 +1552,4 @@ class RealmAuditLog(models.Model): | |||
| event_type = models.CharField(max_length=40) # type: Text | |||
| event_time = models.DateTimeField() # type: datetime.datetime | |||
| backfilled = models.BooleanField(default=False) # type: bool | |||
| extra_data = models.TextField() # type: Text | |||
There was a problem hiding this comment.
I think you should add null=True to this.
fix migrations merge migration fix test fix type annotation fix whitespace
|
@misteraverin I cleaned up the unnecessary "merge" migration by regenerating migrations, added Thanks @misteraverin! |
@timabbott please review. This pull request hasbug with length of extra_data. Sorry for dirty pull request. I don't know why 2 previous commits appear again as they have already been merged into master.