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

fixes install dependencies workflow and lint errors #115

Merged
merged 2 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 5 additions & 4 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: '3.8'
- uses: dschep/install-pipenv-action@v1
- name: Install dependencies
run: pipenv install
python-version: 3.8
- name: Install dependencies with pipenv
run: |
pip install pipenv
pipenv install
- name: Run lint checks
run: pipenv run pylint --load-plugins pylint_django team workshops workshop authentication config noticeboard academics
1 change: 1 addition & 0 deletions academics/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=too-few-public-methods
from django.db import models

# Create your models here.
Expand Down
3 changes: 1 addition & 2 deletions academics/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# pylint: disable=too-few-public-methods
from rest_framework import serializers
from rest_framework.exceptions import NotFound
from drf_yasg2.utils import swagger_serializer_method
from .models import AcademicSchedule, ProfsAndHODs, StudyMaterials


Expand Down
2 changes: 0 additions & 2 deletions academics/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
from django.test import TestCase

# Create your tests here.
14 changes: 6 additions & 8 deletions academics/views.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
from rest_framework import generics, permissions, status
from rest_framework.response import Response
# pylint: disable=no-member
from rest_framework import generics
from django.shortcuts import get_object_or_404
from authentication.models import UserProfile
from .models import AcademicSchedule, StudyMaterials, ProfsAndHODs
from .serializers import (AcademicScheduleSerializer,
ProfsAndHODsSerializer, StudyMaterialsSerializer)
from authentication.models import UserProfile
from rest_framework.permissions import IsAuthenticated, IsAdminUser


class AcademicScheduleView(generics.RetrieveAPIView):
"""
Expand All @@ -31,7 +27,8 @@ def get_object(self):
class StudyMaterialsView(generics.RetrieveAPIView):
'''
Accepts a parameter dept and returns a url of the study materials for the given department.
'dept' is the acronym of the department same as in the email id and contains lower case letters only.
'dept' is the acronym of the department same as in the email id and contains lower case
letters only.
'''

queryset = StudyMaterials.objects.all()
Expand All @@ -43,7 +40,8 @@ class StudyMaterialsView(generics.RetrieveAPIView):
class ProfsAndHODsView(generics.RetrieveAPIView):
'''
Accepts a parameter dept and returns a url of the professors of the given department.
'dept' is the acronym of the department same as in the email id and contains lower case letters only.
'dept' is the acronym of the department same as in the email id and contains lower
case letters only.
'''

queryset = ProfsAndHODs.objects.all()
Expand Down
6 changes: 3 additions & 3 deletions authentication/serializers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# pylint: disable=too-few-public-methods
import logging
from django.core.validators import RegexValidator
# pylint: disable=imported-auth-user
from django.contrib.auth.models import User
from django.contrib.auth import get_user_model
from rest_framework import serializers
from drf_yasg2.utils import swagger_serializer_method
from workshop.serializers import ClubSerializer, EntitySerializer
Expand Down Expand Up @@ -49,7 +49,7 @@ def validate(self, attrs):
raise serializers.ValidationError(
"Please login using @itbhu.ac.in student email id only")
name = jwt['name']
user = User()
user = get_user_model()
Copy link
Member

Choose a reason for hiding this comment

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

@nb9960 Should this be user = get_user_model()() instead? I think this method returns a user model and not the user object.

Something like this should work imo:

UserModel = get_user_model()
user = UserModel()  # or user = UserModel.objects.create_user(...)
...
user.save()

cc: @Vikhyath08
IIT-BHU-InstiApp/IIT-BHU-app#339: Seems like this is a general issue, not related to branch changers.

I fixed a similar bug last time by just reverting the change 😛 (#76), but directly using User might not be good if someone is using custom user model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing out the issue, Recent Commit should probably fix this!

Copy link
Member

Choose a reason for hiding this comment

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

Great! 🎉

user.username = jwt['uid']
user.email = email
user.save()
Expand Down
5 changes: 4 additions & 1 deletion noticeboard/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=too-few-public-methods
# from datetime import date
from rest_framework import serializers
from authentication.models import UserProfile
Expand All @@ -12,6 +13,7 @@ class NoticeDetailSerializer(serializers.ModelSerializer):
has_voted = serializers.SerializerMethodField()

def get_has_voted(self, obj):
"""Check if already voted"""
# pylint: disable=no-member
user = UserProfile.objects.get(user=self.context['request'].user)
# if user in obj.voters.all():
Expand All @@ -22,7 +24,8 @@ def get_has_voted(self, obj):
class Meta:
model = NoticeBoard
read_only_fields = ("id", "upvotes", "downvotes")
fields = ("id", "title", "description", "date", "upvotes", "downvotes","importance", "has_voted")
fields = ("id", "title", "description", "date", "upvotes", "downvotes",
"importance", "has_voted")


class NoticeCreateSerializer(serializers.ModelSerializer):
Expand Down
2 changes: 0 additions & 2 deletions noticeboard/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
from django.test import TestCase

# Create your tests here.
8 changes: 4 additions & 4 deletions noticeboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class NoticeUpvoteView(generics.GenericAPIView):
serializer_class = NoticeDetailSerializer

def get(self, request, pk):
"""Check if already voted or not"""
notice = self.queryset.get(id=pk)
user = UserProfile.objects.get(user=request.user)
if notice is not None:
Expand All @@ -62,8 +63,7 @@ def get(self, request, pk):
return Response(
{"Message": "Upvoted successfully"}, status=status.HTTP_200_OK
)
else:
return Response(
return Response(
{"Error": "Notice not found"}, status=status.HTTP_204_NO_CONTENT
)

Expand All @@ -78,6 +78,7 @@ class NoticeDownvoteView(generics.GenericAPIView):
serializer_class = NoticeDetailSerializer

def get(self, request, pk):
"""Check if already voted or not"""
notice = self.queryset.get(id=pk)
user = UserProfile.objects.get(user=request.user)
if notice is not None:
Expand All @@ -91,7 +92,6 @@ def get(self, request, pk):
return Response(
{"Message": "Downvoted successfully"}, status=status.HTTP_200_OK
)
else:
return Response(
return Response(
{"Error": "Notice not found"}, status=status.HTTP_204_NO_CONTENT
)