Initial models creation by mk-josias · Pull Request #28 · workbud/FriendsWave-api

mk-josias · 2021-11-29T09:38:02Z

The list of models

profil
freinds
group
invitation
userTopic
-member
content
post
comment
like
share
notifications

ttgedeon · 2021-11-29T09:50:40Z

FriendsWave/social/models.py

 from django.db import models
+from django.db.models.base import Model
+from django.db.models.fields.related import OneToOneField
+


General
1- A light comment(specifying purpose) should be added on each field for each model, and a doc-string(purpose/utility) added to each class of the code
2- _Descriptor class is imported form functools module without a clear use
3- Always provide a related name for each Fk and M2M relation
4- Always specify the on_delete behavior on each FK relation
5- use same name to refer to the same reality troughout the code. status vs etat, title vs subject, start vs start_date, end vs end_date
6- Use a Mixins to handle creation, last update time, and soft delete of every

ttgedeon · 2021-11-29T09:58:31Z

FriendsWave/social/models.py

+    etat = models.BooleanField(default=False)
+
+class Friend(models.Model):
+    follower = models.ForeignKey(Profil, OneToOneField = models.CASCADE)


1- Wrong setup of the ForeignKey relations using OneToOneField

ttgedeon · 2021-11-29T10:00:39Z

FriendsWave/social/models.py

+
+class Profil(models.Models):
+    pseudo = models.CharField(max_length=100)
+    image = models.ImageField()


1- Make sure static file settings if well configure and Specify specific storage of each static data with functionnality
2- Consider storing an email on Profile

ttgedeon · 2021-11-29T10:10:29Z

FriendsWave/social/models.py

+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    topic = models.ForeignKey(Topic, on_delete=models.CASCADE)
+    choice_date = models.DateField()
+    raison = models.TextField(max_length=100)


"raison": always use English name to refer to field, variables, ... in code

ttgedeon · 2021-11-29T10:12:50Z

FriendsWave/social/models.py

+class UserTopic(models.Model):
+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    topic = models.ForeignKey(Topic, on_delete=models.CASCADE)
+    choice_date = models.DateField()


choice_date vs creation date use a timestamp model mixins to thandle it automatically in created field

simo97

You have to make some changes on:

coding style (PEP8)
class and attributs naming
foreign keys definitions
and mode

simo97 · 2021-11-29T09:50:05Z

FriendsWave/social/models.py

 from django.db import models
+from django.db.models.base import Model
+from django.db.models.fields.related import OneToOneField
+


You need 2 line between 2 first level code in python (ref PEP 8) https://pep8.org/

simo97 · 2021-11-29T09:50:54Z

FriendsWave/social/models.py

+    etat = models.BooleanField(default=False)
+
+class Friend(models.Model):
+    follower = models.ForeignKey(Profil, OneToOneField = models.CASCADE)


Specify a related_name on this field to avoid duplication on foreignt key definition by the ORM

simo97 · 2021-11-29T09:51:00Z

FriendsWave/social/models.py

+
+class Friend(models.Model):
+    follower = models.ForeignKey(Profil, OneToOneField = models.CASCADE)
+    followed = models.ForeignKey(Profil, OneToOneField = models.CASCADE)


Specify a related_name on this field to avoid duplication on foreignt key definition by the ORM

simo97 · 2021-11-29T09:55:57Z

FriendsWave/social/models.py

+
+class Group(models.Model):
+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)


A field name should not start with _

simo97 · 2021-11-29T09:57:59Z

FriendsWave/social/models.py

+class Group(models.Model):
+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)
+    type = models.BooleanField()


type is a python's reserved word. You can propose aomething alse.

simo97 · 2021-11-29T10:00:07Z

FriendsWave/social/models.py

+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)
+    type = models.BooleanField()
+    created_at = models.DateField() 


The created_at field is intended to be used in what case ? Are you planning to write it value yourself everytime?

simo97 · 2021-11-29T10:02:20Z

FriendsWave/social/models.py

+    members = models.ManyToManyField(Profil, through='Member')
+    private_key = models.TextField(max_length=100, null=True)
+
+class Invitation(models.Model):


Possible foreign key conflit in this class to handle.

There is two Foreign key to the same class.

simo97 · 2021-11-29T10:07:44Z

FriendsWave/social/models.py

+    Content = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    created_at = models.DateField()
+
+class share(models.Model):


Model name don't start like this. Refer to the PEP8 documentation for coding style guideline

simo97 · 2021-11-29T10:08:25Z

FriendsWave/social/models.py

+    end_date = models.DateField(null=True)
+
+class Content(models.Model):
+    Profil = models.ForeignKey(Profil, on_delete=models.CASCADE)


Class attributes don't start like this, refer to the PEP8 coding style.

ttgedeon · 2021-11-29T10:14:59Z

FriendsWave/social/models.py

+    image = models.ImageField()
+    user = models.ForeignKey('auth.User', on_delete=models.CASCADE)
+    friends = models.ManyToManyField('self', through='Friend')
+    topics = models.ManyToManyField('Topic', through='UserTopic')


topics relates to what? topics created by the user, read by the users, deleted by the user.... Use topics_of_interest: it more meaningfull

ttgedeon · 2021-11-29T10:15:38Z

FriendsWave/social/models.py

+
+class Profil(models.Models):
+    pseudo = models.CharField(max_length=100)
+    image = models.ImageField()


user profile_image intead of image: it's more meaningfull

ttgedeon · 2021-11-29T10:17:04Z

FriendsWave/social/models.py

+    user = models.ForeignKey('auth.User', on_delete=models.CASCADE)
+    friends = models.ManyToManyField('self', through='Friend')
+    topics = models.ManyToManyField('Topic', through='UserTopic')
+    etat = models.BooleanField(default=False)


etat field is meaning less. active, is_active, profile_is_active are increasingly meaningful

ttgedeon · 2021-11-29T10:19:27Z

FriendsWave/social/models.py

+    topic = models.ForeignKey(Topic, on_delete=models.CASCADE)
+    choice_date = models.DateField()
+    raison = models.TextField(max_length=100)
+    start_date = models.DateField()


add field like last_opened for practical reasons

ttgedeon · 2021-11-29T10:20:06Z

FriendsWave/social/models.py

+
+
+class Group(models.Model):
+    designation = models.TextField(max_length=100)


consider replacing designation by name

ttgedeon · 2021-11-29T10:20:29Z

FriendsWave/social/models.py

+
+class Group(models.Model):
+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)


remove the underscore on "_description"

ttgedeon · 2021-11-29T10:20:58Z

FriendsWave/social/models.py

+class Group(models.Model):
+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)
+    type = models.BooleanField()


"type" field is meaningless

ttgedeon · 2021-11-29T10:21:44Z

FriendsWave/social/models.py

+    designation = models.TextField(max_length=100)
+    _description = models.TextField(max_length=100)
+    type = models.BooleanField()
+    created_at = models.DateField() 


created_at: should be created, and automatically generated by a Timestamp mixin

ttgedeon · 2021-11-29T10:22:19Z

FriendsWave/social/models.py

+class Invitation(models.Model):
+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    group = models.ForeignKey(Group, on_delete=models.CASCADE)
+    sujbject = models.ForeignKey(Profil, on_delete=models.CASCADE)


ubdate "sujbject"

ttgedeon · 2021-11-29T10:23:30Z

FriendsWave/social/models.py

+    group = models.ForeignKey(Group, on_delete=models.CASCADE)
+    sujbject = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    created_at = models.DateField()
+    etat = models.BooleanField(default=False)


consider storing etat which might refer to state in a charfield with predefined choice list

ttgedeon · 2021-11-29T10:24:59Z

FriendsWave/social/models.py

+    created_at = models.DateField()
+    etat = models.BooleanField(default=False)
+
+class Member(models.Model):


consider adding a state boolean on this class to track multiple times sbdy becomes or leaves his group privileges

ttgedeon · 2021-11-29T10:25:30Z

FriendsWave/social/models.py

+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    is_admin = models.BooleanField(default=False)
+    is_owner = models.BooleanField(default=False)
+    start_date = models.DateField()


start_date should be datetime and handled by a model mixin

ttgedeon · 2021-11-29T10:26:46Z

FriendsWave/social/models.py

+    end_date = models.DateField(null=True)
+
+class Content(models.Model):
+    Profil = models.ForeignKey(Profil, on_delete=models.CASCADE)


1- you are using the "Profil" class name to define a field in an other class
2- No need to link content to Topic class
3- this class should store obvious common attributes between Post and comment leaving any specialized attribute to be defined in corresponding classes

ttgedeon · 2021-11-29T10:29:44Z

FriendsWave/social/models.py

+    class Meta:
+        abstract = True
+
+class Post(Content):


1-a post should be able to store a static file

ttgedeon · 2021-11-29T10:30:15Z

FriendsWave/social/models.py

+    pass
+
+class Comment(Content):
+    post = models.ForeignKey(Post, on_delete=models.CASCADE)


why linking comment to post? what about comment on a comment. review it

ttgedeon · 2021-11-29T10:30:55Z

FriendsWave/social/models.py

+
+class Like(models.Model):
+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    Content = models.ForeignKey(Profil, on_delete=models.CASCADE)


you are using "Content" class name to name a field on an other class

ttgedeon · 2021-11-29T10:31:31Z

FriendsWave/social/models.py

+class Like(models.Model):
+    profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    Content = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    created_at = models.DateField()


created_at should be created, handled with a mixin automatically and must be a datetime

ttgedeon · 2021-11-29T10:32:22Z

FriendsWave/social/models.py

+class share(models.Model):
+    sender = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    recipient = models.ForeignKey(Profil, on_delete=models.CASCADE)
+    post = models.ForeignKey(Post, on_delete=models.CASCADE)


two things can be shared: post & comment. handle this

ttgedeon

Handle all these issues with high precision

…iendsWave-api into feat-modelCorrection

first model correction

models creation

18a52f2

mk-josias requested review from simo97 and ttgedeon November 29, 2021 09:38

mk-josias assigned lepapi-0224 and mk-josias Nov 29, 2021

mk-josias linked an issue Nov 29, 2021 that may be closed by this pull request

Initial models cration #29

Open

ttgedeon reviewed Nov 29, 2021

View reviewed changes

simo97 requested changes Nov 29, 2021

View reviewed changes

ttgedeon reviewed Nov 29, 2021

View reviewed changes

FriendsWave/social/models.py Outdated

class Meta:

abstract = True

class Post(Content):

Copy link

ttgedeon Nov 29, 2021

Choose a reason for hiding this comment

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

1-a post should be able to store a static file

ttgedeon reviewed Nov 29, 2021

View reviewed changes

ttgedeon suggested changes Nov 29, 2021

View reviewed changes

lepapi-0224 and others added 5 commits December 1, 2021 13:53

first model correction

a37f9c8

Merge branch 'feat-modelcreation' into feat-modelCorrection

eba413f

correction requirement

598385b

Merge branch 'feat-modelCorrection' of https://github.com/Workerly/Fr…

15f518d

…iendsWave-api into feat-modelCorrection

Merge pull request #30 from Workerly/feat-modelCorrection

fc44276

first model correction



		class Group(models.Model):
		designation = models.TextField(max_length=100)

Conversation

mk-josias commented Nov 29, 2021

Uh oh!

ttgedeon Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttgedeon Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttgedeon Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simo97 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttgedeon Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttgedeon Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ttgedeon Nov 29, 2021 •

edited

Loading

ttgedeon Nov 29, 2021 •

edited

Loading

ttgedeon Nov 29, 2021 •

edited

Loading

ttgedeon Nov 29, 2021 •

edited

Loading

ttgedeon Nov 29, 2021 •

edited

Loading