Merge branch 'ex3' into 'master'

Refactored Python code

See merge request !5

backend/secfit/comments/permissions.py

@@ -19,3 +19,4 @@ class IsCommentVisibleToUser(permissions.BasePermission):
             or (obj.workout.visibility == "CO" and obj.owner.coach == request.user)
             or obj.workout.owner == request.user
         )
+        
\ No newline at end of file

backend/secfit/comments/views.py

@@ -10,7 +10,9 @@ from rest_framework.filters import OrderingFilter
 
 # Create your views here.
 class CommentList(
-    mixins.ListModelMixin, mixins.CreateModelMixin, generics.GenericAPIView
+    mixins.ListModelMixin,
+    mixins.CreateModelMixin, 
+    generics.GenericAPIView
 ):
     # queryset = Comment.objects.all()
     serializer_class = CommentSerializer
@@ -53,8 +55,14 @@ class CommentList(
                 )
                 | Q(workout__owner=self.request.user)
             ).distinct()
+           
 
-        return qs
+            """for obj in Comment.objects:
+                if IsCommentVisibleToUser.has_object_permission(self, self.request, None, obj):
+                    qs.append(obj)"""
+
+
+        return qs.distinct()
 
 # Details of comment
 class CommentDetail(

backend/secfit/users/admin.py

@@ -6,12 +6,10 @@ from .forms import CustomUserChangeForm, CustomUserCreationForm
 
 # Register your models here.
 
-
 class CustomUserAdmin(UserAdmin):
     add_form = CustomUserCreationForm
     form = CustomUserChangeForm
     model = get_user_model()
-    # list_display = UserAdmin.list_display + ('coach',)
     fieldsets = UserAdmin.fieldsets + ((None, {"fields": ("coach",)}),)
     add_fieldsets = UserAdmin.add_fieldsets + ((None, {"fields": ("coach",)}),)
 

backend/secfit/users/views.py

@@ -59,6 +59,7 @@ class UserDetail(
     queryset = get_user_model().objects.all()
     permission_classes = [permissions.IsAuthenticated & (IsCurrentUser | IsReadOnly)]
 
+    # Is this unecessarily complecated? (lookup_field_options is only a list of two, could make a switch)
     def get_object(self):
         for field in self.lookup_field_options:
             if field in self.kwargs:
@@ -97,8 +98,8 @@ class OfferList(
     def perform_create(self, serializer):
         serializer.save(owner=self.request.user)
 
+    # Nested if-statement and no use of the first queryset varible
     def get_queryset(self):
-        qs = Offer.objects.none()
         result = Offer.objects.none()
 
         if self.request.user:
@@ -106,25 +107,24 @@ class OfferList(
                 Q(owner=self.request.user) | Q(recipient=self.request.user)
             ).distinct()
             qp = self.request.query_params
-            u = self.request.user
+            user = self.request.user
 
             # filtering by status (if provided)
-            s = qp.get("status", None)
-            if s is not None and self.request is not None:
-                qs = qs.filter(status=s)
+            status = qp.get("status", None)
+            category = qp.get("category", None)
+            if status is not None and self.request is not None:
+                qs = qs.filter(status=status)
                 if qp.get("status", None) is None:
-                    qs = Offer.objects.filter(Q(owner=u)).distinct()
+                    qs = Offer.objects.filter(Q(owner=user)).distinct()
 
             # filtering by category (sent or received)
-            c = qp.get("category", None)
-            if c is not None and qp is not None:
-                if c == "sent":
-                    qs = qs.filter(owner=u)
-                elif c == "received":
-                    qs = qs.filter(recipient=u)
+            if category is not None and qp is not None:
+                if category == "sent":
+                    qs = qs.filter(owner=user)
+                elif category == "received":
+                    qs = qs.filter(recipient=user)
             return qs
-        else:
-            return result
+        return result
 
 
 class OfferDetail(

backend/secfit/workouts/models.py

@@ -10,23 +10,6 @@ from django.core.files.storage import FileSystemStorage
 from django.conf import settings
 from django.contrib.auth import get_user_model
 
-
-class OverwriteStorage(FileSystemStorage):
-    """Filesystem storage for overwriting files. Currently unused."""
-
-    def get_available_name(self, name, max_length=None):
-        """https://djangosnippets.org/snippets/976/
-        Returns a filename that's free on the target storage system, and
-        available for new content to be written to.
-
-        Args:
-            name (str): Name of the file
-            max_length (int, optional): Maximum length of a file name. Defaults to None.
-        """
-        if self.exists(name):
-            os.remove(os.path.join(settings.MEDIA_ROOT, name))
-
-
 # Create your models here.
 class Workout(models.Model):
     """Django model for a workout that users can log.

backend/secfit/workouts/serializers.py

@@ -105,7 +105,7 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
 
         for exercise_instance_data in exercise_instances_data:       
             exercise = ExerciseInstance.objects.create(workout=workout, **exercise_instance_data)
-            difficulty += exercise.exercise.difficulty;
+            difficulty += exercise.exercise.difficulty
 
             
         for file_data in files_data:
@@ -140,16 +140,21 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
         instance.visibility = validated_data.get("visibility", instance.visibility)
         instance.date = validated_data.get("date", instance.date)
         
-        
-        difficulty = 0
 
+        self.handle_exercise_instance(instance, exercise_instances_data, exercise_instances)
+        self.handle_workout_files(validated_data, instance)
 
+        return instance
 
         # Handle ExerciseInstances
 
         # This updates existing exercise instances without adding or deleting object.
         # zip() will yield n 2-tuples, where n is
         # min(len(exercise_instance), len(exercise_instance_data))
+    def handle_exercise_instance(self, instance, exercise_instances_data, exercise_instances):
+        """New function to split the long method, manually."""
+        difficulty = 0
+
         for exercise_instance, exercise_instance_data in zip(
             exercise_instances.all(), exercise_instances_data
         ):
@@ -162,7 +167,7 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
             exercise_instance.sets = exercise_instance_data.get(
                 "sets", exercise_instance.sets
             )
-            difficulty += exercise_instance.exercise.difficulty;
+            difficulty += exercise_instance.exercise.difficulty
             exercise_instance.save()
 
         # If new exercise instances have been added to the workout, then create them
@@ -172,7 +177,7 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
                 exercise = ExerciseInstance.objects.create(
                     workout=instance, **exercise_instance_data
                 )
-                difficulty += exercise.exercise.difficulty;
+                difficulty += exercise.exercise.difficulty
         # Else if exercise instances have been removed from the workout, then delete them
         elif len(exercise_instances_data) < len(exercise_instances.all()):
             for i in range(len(exercise_instances_data), len(exercise_instances.all())):
@@ -183,10 +188,11 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
         instance.difficulty = difficulty / len(exercise_instances_data)
 
         instance.save()
-        
-        # Handle WorkoutFiles
-        
-
+    
+    # Handle WorkoutFiles
+    
+    def handle_workout_files(self, validated_data, instance):
+        """Split long method into smaller method, manually """
         if "files" in validated_data:
             files_data = validated_data.pop("files")
             files = instance.files
@@ -207,8 +213,6 @@ class WorkoutSerializer(serializers.HyperlinkedModelSerializer):
                 for i in range(len(files_data), len(files.all())):
                     files.all()[i].delete()
 
-        return instance
-
     def get_owner_username(self, obj):
         """Returns the owning user's username
 

backend/secfit/workouts/views.py

@@ -64,17 +64,18 @@ class RememberMe(
 
     serializer_class = RememberMeSerializer
 
+    # Code smell - unnecessary else statement
     def get(self, request):
-        if request.user.is_authenticated == False:
+        if not request.user.is_authenticated:
             raise PermissionDenied
-        else:
-            return Response({"remember_me": self.rememberme()})
+        return Response({"remember_me": self.rememberme()})
 
+    # Change in name "cookieObjects"
     def post(self, request):
-        cookieObject = namedtuple("Cookies", request.COOKIES.keys())(
+        cookie_object = namedtuple("Cookies", request.COOKIES.keys())(
             *request.COOKIES.values()
         )
-        user = self.get_user(cookieObject)
+        user = self.get_user(cookie_object)
         refresh = RefreshToken.for_user(user)
         return Response(
             {
@@ -83,22 +84,26 @@ class RememberMe(
             }
         )
 
-    def get_user(self, cookieObject):
-        decode = base64.b64decode(cookieObject.remember_me)
+    # Needs change of return statement
+    def get_user(self, cookie_object):
+        decode = base64.b64decode(cookie_object.remember_me)
         user, sign = pickle.loads(decode)
 
         # Validate signature
         if sign == self.sign_user(user):
             return user
+        # return None
 
     def rememberme(self):
+        """Remember the user"""
         creds = [self.request.user, self.sign_user(str(self.request.user))]
         return base64.b64encode(pickle.dumps(creds))
 
+    #Changed this!
     def sign_user(self, username):
+        "Makes a signer"
         signer = Signer()
-        signed_user = signer.sign(username)
-        return signed_user
+        return signer.sign(username)
 
 
 class WorkoutList(