WIP: Changed annotation syntax to properties and created mutable user_properties #31

Closed
rskthomas wants to merge 27 commits from rework/properties into main
Showing only changes of commit 41926ea85a - Show all commits

View file

@ -3,13 +3,15 @@ import json
from dmidecode import DMIParse
from django.db import models
from django.db.models import Q
from utils.constants import STR_EXTEND_SIZE, CHASSIS_DH
from evidence.xapian import search
from evidence.parse_details import ParseSnapshot
from user.models import User, Institution
class Annotation(models.Model):
#TODO: base class is abstract; revise if should be for query efficiency
class Property(models.Model):
Outdated
Review

La clase Type no tiene sentido que este en Property. Creo que es mejor ponerla en la clase UserProperty. Lo mismo con la columna type. Solo pondria la columna type en UserProperty. Esto te libera del CheckConstraint de los dos modelos que has creado.
El CheckConstraint lo puedes cambiar en SystemProperty haciendo que solo coja los fields key, uuid

La clase Type no tiene sentido que este en Property. Creo que es mejor ponerla en la clase UserProperty. Lo mismo con la columna type. Solo pondria la columna type en UserProperty. Esto te libera del CheckConstraint de los dos modelos que has creado. El CheckConstraint lo puedes cambiar en SystemProperty haciendo que solo coja los fields key, uuid
class Type(models.IntegerChoices):
SYSTEM = 0, "System"
USER = 1, "User"
@ -30,6 +32,30 @@ class Annotation(models.Model):
models.UniqueConstraint(
fields=["type", "key", "uuid"], name="unique_type_key_uuid")
Outdated
Review

En general separo las clases con dos lineas y los metodos de la clase solo con una linea
Si un fichero no tiene clases y solo tiene funciones, entonces separo esas funciones con dos lineas

En general separo las clases con dos lineas y los metodos de la clase solo con una linea Si un fichero no tiene clases y solo tiene funciones, entonces separo esas funciones con dos lineas

Perfecto! Lo tendré en cuenta a partir de ahora y lo cambio en donde vea que haga falta

Perfecto! Lo tendré en cuenta a partir de ahora y lo cambio en donde vea que haga falta
]
abstract = True
class SystemProperty(Property):
class Meta:
constraints = [
models.CheckConstraint(
check=~Q(type=1), #Enforce that type is not User
name='property_cannot_be_user'
),
]
class UserProperty(Property):
type = models.SmallIntegerField(default=Property.Type.USER)
class Meta:
constraints = [
models.CheckConstraint(
check=Q(type=1), #Enforce that type is User
name='property_needs_to_be_user'
Outdated
Review

Esto desaparece con lo que no tienes que preocuparte. Pero fíjate que has puesto dos variables con el mismo nombre en la clase Meta. Osea que has redefinido una variable y una de las dos no se usará. Para otra ocasión creo que seria más conveniente poner todo dentro de una única variable constraints ya que de por si es una lista. Podrías haber hecho uso de los poderes de la lista añadiéndolo como un elemento más. Lo dicho, no te preocupes por esto ahora porque esto tiene que desaparecer.

Esto desaparece con lo que no tienes que preocuparte. Pero fíjate que has puesto dos variables con el mismo nombre en la clase Meta. Osea que has redefinido una variable y una de las dos no se usará. Para otra ocasión creo que seria más conveniente poner todo dentro de una única variable constraints ya que de por si es una lista. Podrías haber hecho uso de los poderes de la lista añadiéndolo como un elemento más. Lo dicho, no te preocupes por esto ahora porque esto tiene que desaparecer.

La verdad que sí, fue un error tan tonto que no sé como no lo ví. Estoy haciendo el refactoring como comentaste más arriba así no hace falta dos constraints

La verdad que sí, fue un error tan tonto que no sé como no lo ví. Estoy haciendo el refactoring como comentaste más arriba así no hace falta dos constraints
),
]
class Evidence: