Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
89 changes: 88 additions & 1 deletion FriendsWave/social/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,90 @@
from functools import _Descriptor
from django.db import models
from django.db.models.base import Model
from django.db.models.fields.related import OneToOneField

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@ttgedeon ttgedeon Nov 29, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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

@ttgedeon ttgedeon Nov 29, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

user profile_image intead of image: it's more meaningfull

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

etat = models.BooleanField(default=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1- Wrong setup of the ForeignKey relations using OneToOneField

followed = models.ForeignKey(Profil, OneToOneField = models.CASCADE)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

status = models.BooleanField()

class Topic(models.Model):
subject = models.TextField(max_length=100)
description = models.TextField(max_length=100)
creator = models.ForeignKey(Profil, on_delete=models.SET_NULL)

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

raison = models.TextField(max_length=100)

@ttgedeon ttgedeon Nov 29, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

start_date = models.DateField()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

add field like last_opened for practical reasons

end_date = models.DateField(null=True)


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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

consider replacing designation by name

_description = models.TextField(max_length=100)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A field name should not start with _

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

remove the underscore on "_description"

type = models.BooleanField()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"type" field is meaningless

created_at = models.DateField()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

members = models.ManyToManyField(Profil, through='Member')
private_key = models.TextField(max_length=100, null=True)

class Invitation(models.Model):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possible foreign key conflit in this class to handle.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is two Foreign key to the same class.

profil = models.ForeignKey(Profil, on_delete=models.CASCADE)
group = models.ForeignKey(Group, on_delete=models.CASCADE)
sujbject = models.ForeignKey(Profil, on_delete=models.CASCADE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ubdate "sujbject"

created_at = models.DateField()
etat = models.BooleanField(default=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


class Member(models.Model):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

start_date should be datetime and handled by a model mixin

end_date = models.DateField(null=True)

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@ttgedeon ttgedeon Nov 29, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

topic = models.ForeignKey(Topic, on_delete=models.CASCADE)
title = models.TextField()
content = models.TextField()

class Meta:
abstract = True

class Post(Content):

Copy link
Copy Markdown

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

pass

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

@ttgedeon ttgedeon Nov 29, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

created_at = models.DateField()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


class share(models.Model):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

sender = models.ForeignKey(Profil, on_delete=models.CASCADE)
recipient = models.ForeignKey(Profil, on_delete=models.CASCADE)
post = models.ForeignKey(Post, on_delete=models.CASCADE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


class Notification(models.Model):
sender = models.ForeignKey(Profil, on_delete=models.CASCADE)
recipient = models.ForeignKey(Profil, on_delete=models.CASCADE)
created_at = models.DateField()
title = models.TextField(max_length=50)
message = models.TextField(max_length=100)




# Create your models here.
1 change: 0 additions & 1 deletion FriendsWave/social/views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
from django.shortcuts import render

# Create your views here.