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.

Sign up for GitHub

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

Change actstream models ids type from Char to UUID #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Change actstream models ids type from Char to UUID #1

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
026e3e7
chore: change ids type from char to uuid in actstream models
Vaibhav015 Dec 26, 2019
ac12f3f
chore: add data migration for converting ids type from char to uuid i…
Vaibhav015 Dec 26, 2019
2e4a52e
fix: remove default and max length from uuid field type in actstream …
Vaibhav015 Dec 27, 2019
ab362d9
fix: actstream migration
Vaibhav015 Dec 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions actstream/migrations/0004_auto_20191226_0744.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Generated by Django 2.1.11 on 2019-12-26 07:44

from django.db import migrations, models
import uuid


class Migration(migrations.Migration):

dependencies = [
('actstream', '0003_add_follow_flag'),
]

operations = [
migrations.AlterField(
model_name='action',
name='action_object_object_id',
field=models.UUIDField(blank=True, db_index=True, default=uuid.uuid4, null=True),
),
migrations.AlterField(
model_name='action',
name='actor_object_id',
field=models.UUIDField(db_index=True, default=uuid.uuid4),
),
migrations.AlterField(
model_name='action',
name='target_object_id',
field=models.UUIDField(blank=True, db_index=True, default=uuid.uuid4, null=True),
),
migrations.AlterField(
model_name='follow',
name='object_id',
field=models.UUIDField(db_index=True, default=uuid.uuid4),
),
]
14 changes: 8 additions & 6 deletions actstream/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import unicode_literals

import uuid

from django.conf import settings
from django.contrib.contenttypes.fields import GenericForeignKey
from django.db import models
Expand All @@ -24,7 +26,7 @@ class Follow(models.Model):
content_type = models.ForeignKey(
ContentType, on_delete=models.CASCADE, db_index=True
)
object_id = models.CharField(max_length=255, db_index=True)
object_id = models.UUIDField(max_length=255, db_index=True, default=uuid.uuid4)
Copy link
Author

Choose a reason for hiding this comment

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

@Vaibhav015 i think we do not need to add default=uuid.uuid4

Copy link
Author

Choose a reason for hiding this comment

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

Also, why have we given max_length in uuid field ? I mean do not require this ?

Copy link

Choose a reason for hiding this comment

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

As per docs, It's recommended to use default in UUIDField @veerat-pdgt
Ref Docs: https://docs.djangoproject.com/en/3.0/ref/models/fields/#uuidfield
cc @vikalpj
Let me know your thoughts @veerat-pdgt @vikalpj 🙂

Copy link

Choose a reason for hiding this comment

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

@vaibhav-pdgt The case mentioned in doc is only applicable when we have PK for uuid. else default should always be based on the usecase. :)

Copy link

Choose a reason for hiding this comment

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

yeah, thanks @vikalpj

Copy link

Choose a reason for hiding this comment

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

fixes done

follow_object = GenericForeignKey()
actor_only = models.BooleanField(
"Only follow actions where "
Expand Down Expand Up @@ -75,7 +77,7 @@ class Action(models.Model):
ContentType, related_name='actor',
on_delete=models.CASCADE, db_index=True
)
actor_object_id = models.CharField(max_length=255, db_index=True)
actor_object_id = models.UUIDField(max_length=255, db_index=True, default=uuid.uuid4)
actor = GenericForeignKey('actor_content_type', 'actor_object_id')

verb = models.CharField(max_length=255, db_index=True)
Expand All @@ -86,8 +88,8 @@ class Action(models.Model):
related_name='target',
on_delete=models.CASCADE, db_index=True
)
target_object_id = models.CharField(
max_length=255, blank=True, null=True, db_index=True
target_object_id = models.UUIDField(
max_length=255, blank=True, null=True, db_index=True, default=uuid.uuid4
)
target = GenericForeignKey(
'target_content_type',
Expand All @@ -99,8 +101,8 @@ class Action(models.Model):
related_name='action_object',
on_delete=models.CASCADE, db_index=True
)
action_object_object_id = models.CharField(
max_length=255, blank=True, null=True, db_index=True
action_object_object_id = models.UUIDField(
max_length=255, blank=True, null=True, db_index=True, default=uuid.uuid4
)
action_object = GenericForeignKey(
'action_object_content_type',
Expand Down