Some issues found while testing

[nishantwrp]

• POST /tags/create/ - It is mentioned that any workshop contact can create a tag. But I got a 400 error. Can you confirm that this is a bug or intentional? And if the user is not authenticated there should be a 403 error not 400. Maybe we should move this endpoint to /clubs/{id}/tags/create/.

• GET /councils/{id}/ - This is a sample response

  {
  "id": 1,
  "name": "council_name",
  "description": "council_description",
  "gensec": {
  "id": 1,
  "name": "Secy Name",
  "email": "secy.cse18@itbhu.ac.in",
  "phone_number": "1234567890",
  "photo_url": "https://secy.com/"
  },
  "joint_gensec": [
  {
    "id": 1,
    "name": "Secy Name",
    "email": "secy.cse18@itbhu.ac.in",
    "phone_number": "1234567890",
    "photo_url": "https://secy.com/"
  }
  ],
  "clubs": [
  {
    "id": 1,
    "name": "club_name",
    "council": {
      "id": 1,
      "name": "council_name",
      "small_image_url": "https://council-small_image.com/",
      "large_image_url": "https://council-large_image.com/"
    },
    "small_image_url": "https://club-small_image.com/",
    "large_image_url": "https://club-large_image.com/"
  }
  ],
  "small_image_url": "https://council-small_image.com/",
  "large_image_url": "https://council-large_image.com/",
  "website_url": "https://council-website.com/",
  "facebook_url": "https://council-facebook.com/",
  "twitter_url": "https://council-twitter.com/",
  "instagram_url": "https://council-instagram.com/",
  "linkedin_url": "https://council-linkedin.com/",
  "youtube_url": "https://council-youtube.com/"
  }

Now here in clubs we have council again. Doesn't it seem weird?

/cc @krashish8

[krashish8]

The tags can be created only for the club in which the user is a Workshop Contact. Are you trying to create a tag for another club? @nishantwrp

[nishantwrp]

No for the same club

[krashish8]

And since this is created due to serializers.ValidationError, it return a 400 error. We can either make changes in permissions.py file, or can replace serializers.ValidationError with PermissionDenied in the required line. Which one would you prefer? @nishantwrp

[krashish8]

No for the same club

Oh. Let me check.

[krashish8]

What was the exact error message?

[krashish8]

• POST /tags/create/ - It is mentioned that any workshop contact can create a tag. But I got a 400 error. Can you confirm that this is a bug or intentional? And if the user is not authenticated there should be a 403 error not 400. Maybe we should move this endpoint to /clubs/{id}/tags/create/.

/cc @krashish8

Yes! I was also thinking to move this to /clubs/{id}/tags/create/. The endpoint /workshops/create/ can also be moved to /clubs/{id}/workshops/create/. What's your opinion? @nishantwrp

[nishantwrp]

Yup, I'm in favour of this.

[nishantwrp]

What was the exact error message?

"You are not allowed to create tag for this club"

[krashish8]

Now here in clubs we have council again. Doesn't it seem weird?

Yes, we can change this. Do you know a better way to remove certain fields from a nested serializer, without creating another serializer? Here, we need to remove the council field from the ClubSerializer in the CouncilDetailSerializer class. @nishantwrp

[krashish8]

What was the exact error message?

"You are not allowed to create tag for this club"

Found the error! In Line 220, club not in profile.get_workshop_privileges().values_list('club', flat=True)): should be changed to club.id not in profile.get_workshop_privileges().values_list('club', flat=True)):

[krashish8]

Also, this line raise serializers.ValidationError("You are not allowed to create tag for this club") will return a 400 Error. Shall we change it to 403 here, and in similar places? But as a good practice, 403 error should not be raised in serializers, but in permissions. Any suggestions on how to do that?

[nishantwrp]

Ok, I think this can be done when we refactor the endpoint. Can you create a different issue for things that can be refactored like

• Tag Creation Bug
• All endpoints must return 403 if there is a authorization error.
• Move /tags/create to /clubs/{id}/tags/create

Ans so, on.

[nishantwrp]

Now here in clubs we have council again. Doesn't it seem weird?

Yes, we can change this. Do you know a better way to remove certain fields from a nested serializer, without creating another serializer? Here, we need to remove the council field from the ClubSerializer in the CouncilDetailSerializer class. @nishantwrp

Nope. I don't. @shivanshs9 Do you?

[nishantwrp]

@krashish8 Also if we decide to make a new serializer, thinks may go complicated. How about creating directory serializers and files like clubs.py where we can have all serializers related to clubs and so on?

[krashish8]

@krashish8 Also if we decide to make a new serializer, thinks may go complicated. How about creating directory serializers and files like clubs.py where we can have all serializers related to clubs and so on?

Hmm... this can be done.

[shivanshs9]

Now here in clubs we have council again. Doesn't it seem weird?

Yes, we can change this. Do you know a better way to remove certain fields from a nested serializer, without creating another serializer? Here, we need to remove the council field from the ClubSerializer in the CouncilDetailSerializer class. @nishantwrp

Nope. I don't. @shivanshs9 Do you?

Yup, it can be done. Here's the mixin base class which I use for this purpose:

class DynamicFieldExclude(object):
    """
    Inherit from this class in any sub-class of ModelSerializer to
    support dynamic fields exclusion in any serializer when creating
    serializer object.
    """

    def __init__(self, *args, **kwargs):
        exclude_fields = kwargs.pop('exclude', None)
        super(DynamicFieldExclude, self).__init__(*args, **kwargs)

        if exclude_fields:
            for field_name in exclude_fields:
                self.fields.pop(field_name, None)