Skip to content

PR: Added Frontend (React 18) and Backend (Django REST Framework) for Heritage Graph#10

Open
nabin2004 wants to merge 25 commits into
mainfrom
django
Open

PR: Added Frontend (React 18) and Backend (Django REST Framework) for Heritage Graph#10
nabin2004 wants to merge 25 commits into
mainfrom
django

Conversation

@nabin2004

Copy link
Copy Markdown
Member

Description

This PR adds the structure for the Heritage graph application by adding both the frontend and backend directories. The frontend directory contains the React 18 application, while the backend directory includes the Django REST Framework setup for the core application.

Changes Made:

  • Added a frontend directory containing the React 18 application setup.
  • Added a backend directory with the Django REST Framework for the API.

@nabin2004 nabin2004 changed the title PR: Add Frontend (React 18) and Backend (Django REST Framework) for Heritage Graph PR: Added Frontend (React 18) and Backend (Django REST Framework) for Heritage Graph Dec 18, 2024
@semihyumusak

Copy link
Copy Markdown

It is a very large PR. In order to have a review, I need to run it. There are no installation documentation in the repo.

In this PR the only thing I see is;

  • No need to include migration files as they are automatically generated
  • The same for the sqlite db file.

@nabin2004

Copy link
Copy Markdown
Member Author

It is a very large PR. In order to have a review, I need to run it. There are no installation documentation in the repo.

In this PR the only thing I see is;

* No need to include migration files as they are automatically generated

* The same for the sqlite db file.

Resolved!

@tekrajchhetri

Copy link
Copy Markdown
Member

@nabin2004 can you fix these conflict?

@tekrajchhetri tekrajchhetri left a comment

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.

Please fix the merge conflict.

Comment thread backend/heritage_graph/db.sqlite3 Outdated

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.

@nabin2004 @nirajkark is there a reason why we need to push the database?



class Migration(migrations.Migration):

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.

It would be nice to add the comments to describe what this code is.

    """
    Migration for heritage data models.
    Creates:
    - Submission model with .....
    Establishes relationships between users and submissions
    """

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.

this is also a best practices

@@ -0,0 +1,35 @@
from django.contrib import admin

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.

@nabin2004 @nirajkark I would suggest adding something like shown below at the beginning of the code. I have configured the pycharm to do so. Whenever I create a new python code following is automatically added.

# -*- coding: utf-8 -*-
# -----------------------------------------------------------------------------
# DISCLAIMER: This software is provided "as is" without any warranty,
# express or implied, including but not limited to the warranties of
# merchantability, fitness for a particular purpose, and non-infringement.
#
# In no event shall the authors or copyright holders be liable for any
# claim, damages, or other liability, whether in an action of contract,
# tort, or otherwise, arising from, out of, or in connection with the
# software or the use or other dealings in the software.
# -----------------------------------------------------------------------------

# @Author  : <author-name>
# @Email   : <cair-nepal-email-address>
# @File    : <filename>
# @Software: PyCharm

]
title = models.CharField(max_length=255)
description = models.TextField()
contributor = models.ForeignKey(User, on_delete=models.CASCADE, related_name='submissions')

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.

@nabin2004 We should not delete the submission if user is deleted for any reason. Remove the CASCADE option and change it to appropriate one.

return f"{self.title} ({self.get_status_display()})"

class Moderation(models.Model):
submission = models.OneToOneField(Submission, on_delete=models.CASCADE, related_name='moderation')

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.

@nabin2004 @nirajkark We don't want cascading, please adjust accordingly

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.

@nabin2004 @nirajkark

  • Add the docstrings for all the code



class UserProfileAdmin(admin.ModelAdmin):
list_display = ('user', 'organization', 'score','position','birth_date','university_school')

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.

Add list filter options.

migrations.AlterField(
model_name='moderation',
name='submission',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='moderations', to='heritage_data.submission'),

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.

@nirajkark @nabin2004 Update django.db.models.deletion.CASCADE, we don't want to 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.

use PROTECT

# See https://docs.djangoproject.com/en/5.1/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = 'django-insecure-03eebp+*3833bj!9v)r41dvnv8eg%#avt!eyq7s=kk8@&plg^$'

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.

@nabin2004 @nirajkark use the environment variable for the key and do not hardcode it.

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 can use https://github.com/HBNetwork/python-decouple

Also following is the suggested security update.

from decouple import config

SECRET_KEY = config('DJANGO_SECRET_KEY')
DEBUG = config('DEBUG', default=False, cast=bool)

CORS_ALLOWED_ORIGINS = config('CORS_ORIGINS', default='').split(',')

REST_FRAMEWORK = {
    'DEFAULT_AUTHENTICATION_CLASSES': [
        'rest_framework.authentication.SessionAuthentication',
        'rest_framework.authentication.TokenAuthentication',
    ],
    'DEFAULT_PERMISSION_CLASSES': [
        'rest_framework.permissions.IsAuthenticated',
    ],
    'DEFAULT_THROTTLE_CLASSES': [
        'rest_framework.throttling.AnonRateThrottle',
        'rest_framework.throttling.UserRateThrottle'
    ],
    'DEFAULT_THROTTLE_RATES': {
        'anon': '100/day',
        'user': '1000/day'
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants