Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Generated by Django 4.2.15 on 2024-11-05 07:02

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("db", "0083_device_workspace_timezone_and_more"),
]

operations = [
migrations.RemoveConstraint(
model_name="label",
name="label_unique_name_project_when_deleted_at_null",
),
migrations.AlterUniqueTogether(
name="label",
unique_together=set(),
),
migrations.AddField(
model_name="deployboard",
name="is_disabled",
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name="inboxissue",
name="extra",
field=models.JSONField(default=dict),
),
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix mutable default in JSONField.

Using default=dict for JSONField is dangerous as it creates a mutable default that's shared between instances.

Replace with:

-field=models.JSONField(default=dict),
+field=models.JSONField(default=dict()),

Committable suggestion skipped: line range outside the PR's diff.

migrations.AddField(
model_name="inboxissue",
name="source_email",
field=models.TextField(blank=True, null=True),
),
migrations.AddField(
model_name="user",
name="bot_type",
field=models.CharField(
blank=True, max_length=30, null=True, verbose_name="Bot Type"
),
),
migrations.AlterField(
model_name="deployboard",
name="entity_name",
field=models.CharField(blank=True, max_length=30, null=True),
),
migrations.AlterField(
model_name="inboxissue",
name="source",
field=models.CharField(
blank=True, default="IN_APP", max_length=255, null=True
),
),
Comment on lines +48 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add choices for source field.

The source field should have predefined choices to maintain data consistency, especially since "IN_APP" is a default value.

Consider adding choices like:

SOURCE_CHOICES = [
    ("IN_APP", "In App"),
    ("EMAIL", "Email"),
    # add other sources
]

migrations.AddConstraint(
model_name="label",
constraint=models.UniqueConstraint(
condition=models.Q(
("deleted_at__isnull", True), ("project__isnull", True)
),
fields=("name",),
name="unique_name_when_project_null_and_not_deleted",
),
),
migrations.AddConstraint(
model_name="label",
constraint=models.UniqueConstraint(
condition=models.Q(
("deleted_at__isnull", True), ("project__isnull", False)
),
fields=("project", "name"),
name="unique_project_name_when_not_deleted",
),
),
]
5 changes: 4 additions & 1 deletion apiserver/plane/db/models/deploy_board.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ class DeployBoard(WorkspaceBaseModel):
("cycle", "Task"),
("page", "Page"),
("view", "View"),
("intake", "Intake"),
)

entity_identifier = models.UUIDField(null=True)
entity_name = models.CharField(
max_length=30,
choices=TYPE_CHOICES,
null=True,
blank=True,
Comment on lines +29 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Restore type safety for entity_name field

The removal of choices=TYPE_CHOICES while adding null=True, blank=True introduces several concerns:

  1. Loss of type safety - values are no longer validated against TYPE_CHOICES
  2. Potential conflict with unique constraints when null values are introduced
  3. No validation mechanism to ensure values align with defined types

Consider one of these solutions:

entity_name = models.CharField(
    max_length=30,
-   null=True,
-   blank=True,
+   choices=TYPE_CHOICES,
+   null=True,  # if null values are really needed
+   blank=True, # if blank values are really needed
)

Or if flexible values are required, add a validator:

+from django.core.validators import RegexValidator
+
entity_name = models.CharField(
    max_length=30,
    null=True,
    blank=True,
+   validators=[RegexValidator(
+       regex='^(project|issue|module|cycle|page|view|intake)$',
+       message='entity_name must match one of the defined types',
+   )]
)

Committable suggestion skipped: line range outside the PR's diff.

)
anchor = models.CharField(
max_length=255, default=get_anchor, unique=True, db_index=True
Expand All @@ -41,6 +43,7 @@ class DeployBoard(WorkspaceBaseModel):
is_votes_enabled = models.BooleanField(default=False)
view_props = models.JSONField(default=dict)
is_activity_enabled = models.BooleanField(default=True)
is_disabled = models.BooleanField(default=False)

def __str__(self):
"""Return name of the deploy board"""
Expand Down
9 changes: 8 additions & 1 deletion apiserver/plane/db/models/inbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,16 @@ class InboxIssue(ProjectBaseModel):
on_delete=models.SET_NULL,
null=True,
)
source = models.TextField(blank=True, null=True)
source = models.CharField(
max_length=255,
default="IN_APP",
null=True,
blank=True,
)
source_email = models.TextField(blank=True, null=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider JSONField default value implementation.

While the new fields are well-structured, using a mutable default (dict) for JSONField can lead to unexpected behavior. Consider using a callable default instead.

-    extra = models.JSONField(default=dict)
+    extra = models.JSONField(default=dict)
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if self.extra is None:
+            self.extra = {}

Also, consider adding validation for the source_email field if it's intended to store email addresses:

+    def clean(self):
+        super().clean()
+        if self.source_email:
+            from django.core.validators import validate_email
+            validate_email(self.source_email)

Also applies to: 69-69

external_source = models.CharField(max_length=255, null=True, blank=True)
external_id = models.CharField(max_length=255, blank=True, null=True)
extra = models.JSONField(default=dict)

class Meta:
verbose_name = "InboxIssue"
Expand Down
16 changes: 11 additions & 5 deletions apiserver/plane/db/models/label.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@ class Label(WorkspaceBaseModel):
external_id = models.CharField(max_length=255, blank=True, null=True)

class Meta:
unique_together = ["name", "project", "deleted_at"]
constraints = [
# Enforce uniqueness of name when project is NULL and deleted_at is NULL
models.UniqueConstraint(
fields=["name", "project"],
condition=Q(deleted_at__isnull=True),
name="label_unique_name_project_when_deleted_at_null",
)
fields=["name"],
condition=Q(project__isnull=True, deleted_at__isnull=True),
name="unique_name_when_project_null_and_not_deleted",
),
# Enforce uniqueness of project and name when project is not NULL and deleted_at is NULL
models.UniqueConstraint(
fields=["project", "name"],
condition=Q(project__isnull=False, deleted_at__isnull=True),
name="unique_project_name_when_not_deleted",
),
]
verbose_name = "Label"
verbose_name_plural = "Labels"
Expand Down
6 changes: 6 additions & 0 deletions apiserver/plane/db/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class User(AbstractBaseUser, PermissionsMixin):
# my_issues_prop = models.JSONField(null=True)

is_bot = models.BooleanField(default=False)
bot_type = models.CharField(
max_length=30,
verbose_name="Bot Type",
blank=True,
null=True,
)

# timezone
USER_TIMEZONE_CHOICES = tuple(zip(pytz.all_timezones, pytz.all_timezones))
Expand Down