Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/BE-4 #190

Merged
merged 5 commits into from
Oct 23, 2022
Merged

Feature/BE-4 #190

merged 5 commits into from
Oct 23, 2022

Conversation

BElifb
Copy link
Contributor

@BElifb BElifb commented Oct 23, 2022

  • Please clean your database and delete any previous migrations you may have, since extending the AbstractUser will change the whole database schema.
  • Created User class by extending AbstractUser, also created Tag, ArtItem and Comment classes.
  • Added AUTH_USER_MODEL to settings.py. Also, prefer to create foreign keys to the User model importing the settings from django.conf import settings and referring to the settings.AUTH_USER_MODEL instead of referring directly to the custom User model.
  • Installed Pillow package for the media files and updated requirements.txt with the latest additions.
  • Configured MEDIA_ROOT in settings.py and added MEDIA folder with default images.
  • Created serializers for the models.
  • Created a decorator to check whether the user is active_user or not. There are some things to keep in mind here:
    • Current redirection path for the decorator is 'login', but we can configure this later as the project structure is more defined.
    • is_active and is_active_user are two seperate concepts. First refers to user's account not being blocked (we'll use this when there are complaints about the account and the admin/moderator decides to ban the user), second (is_active_user) refers to user having a level 2 acoount.
  • Created and configured the admin site.
  • Tested creating users via admin site, fixed media root.
  • Please refer to the BE-4: Setting up initial models #187 for further information

@BElifb BElifb requested review from KarahanS and mumcusena October 23, 2022 07:15
@KarahanS
Copy link
Contributor

KarahanS commented Oct 23, 2022

Could you have a look at my comment about settings in #187?

Edit: We decided to merge this PR first and fix the other one if we encounter with any pr conflicts.

@BElifb
Copy link
Contributor Author

BElifb commented Oct 23, 2022

  • Created basic homepage.
  • Added base and index templates to newly created folder in API app.
  • Added redirection to admin site.

@KarahanS
Copy link
Contributor

Looks good to me. Right now, media doesn't seem to have any functionality (Django handles the images I guess) - but it will be useful when we want to receive an image from Frontend in base-64 encoded version, decode it and save it in the media folder.

@KarahanS KarahanS added the Approved This work is reviewed and approved by a team member label Oct 23, 2022
@KarahanS KarahanS added Status: Review Needed A review is needed for this issue and removed Approved This work is reviewed and approved by a team member labels Oct 23, 2022
@KarahanS
Copy link
Contributor

KarahanS commented Oct 23, 2022

I re-configured the settings folder to include both development and production settings. Can I kindly request you to move your .env file to the settings folder and run the application to see if everything is working properly? (It's working on my side but let's make sure that it's working in general).
I'm taking the PR to Review needed, however you can merge it directly if you confirm that new settings is working properly.

(I'm adding this configuration to make sure that everyone starts with the same settings configuration before writing their APIs)

@KarahanS KarahanS added Approved This work is reviewed and approved by a team member and removed Status: Review Needed A review is needed for this issue labels Oct 23, 2022
@BElifb
Copy link
Contributor Author

BElifb commented Oct 23, 2022

  • Profile images in the Media folder were giving an error. I updated the BASE_DIR in base.py, since the depth of folders has increased. Everything seems to be working now. You can check it a final time @KarahanS , otherwise I'm merging it to master.

@KarahanS
Copy link
Contributor

LGTM

@BElifb BElifb merged commit 71f21bb into master Oct 23, 2022
@BElifb BElifb deleted the feature/BE-4 branch October 23, 2022 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved This work is reviewed and approved by a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants