Add Teams Endpoints to manage teams

[tdrivas]

New endpoints have been created for managing both teams and member under organization for CRUD actions

[tdrivas]

Planned Tasks

• Task 1: Create missing serializers for each entity
• Task 2: Create tests for each case

[suricactus]

Hey! There are a few things cleanup to be done before you continue working on these. Would you mind to address the feedback first, so it will be much easier for review.

In short:

• don't leave random commented code here and there
• no need to comment obvious things
• please install pre-commit
• please rebase on master
• remove docker-compose.override.local.yml changes

[suricactus]

General comment:

• there are still archeological treasures from the initial development, please make sure you remove these and push them
• merge all the tests in one test_teams.py file
• the views can be a bit more django-rest-framework-like (DRF) , e.g. you can use RetrieveUpdateDestroyAPIView or similar for the views baseclasses for "teams" and "team members". This will greatly reduce your codebase and simplify maintainence.

1. cleanup the initial comments in this PR and push all the changes.
2. rebase to the latest master and push
3. run pre-commit and push
4. address most of the requests not related the the views.py or tests
5. try to automate views behavior with DRF
6. continue with tests

[suricactus]

Can you please rebase on the latest master and squash all the changes in a single commit?

Please add typing on all the methods that you add in this PR, except the tests.

Also, unfortunately I see some of our verbal conversation feedback was not addressed.

[suricactus]

You are not adding a user, I guess the message should be different.

Suggested change

	raise serializers.ValidationError(
	{
	"member": f"A user with username '{email_or_username}' already exists."
	}
	)
	raise serializers.ValidationError(
	{
	"member": f"A member with username '{email_or_username}' already exists."
	}
	)

[tdrivas]

This comments is answering all the comments above related to function internal_value:

The idea is the following:

1. check if the username of the member that is going to be added already exists in another member
2. check if member is indeed a user (this might be overkill due to an existing check via permissions in the view(???)
3. otherwise use the fast_search to obtain the user

  try:
            existing_user = User.objects.fast_search(email_or_username)
            
            if TeamMember.objects.filter(member=existing_user):
                raise serializers.ValidationError(
                        {
                            "member": f"A member with username '{email_or_username}' already exists."
                        }
                )
            
            validated_data["member"] = existing_user
        
        except User.DoesNotExist:  
            
            raise serializers.ValidationError(
                        {
                            "member": f"A user with username '{email_or_username}' does not exists."
                        }
            )
        
        return validated_data

[suricactus]

What happens if we don't explicitly handle the DoesNotExist error, or we attempt to break the uniqueness of the members in the organization? Do we get a proper error or just error 500? What is the HTTP response body?

If we get 500, then we need to handle it. Your proposal is good.

Note that the messages are weird, you can get a validation message like: "A member with username [email protected] already exists". Just say 'The team member "[email protected]" already exists." We always put placeholders in double quotes in user strings.

[tdrivas]

Can you please rebase on the latest master and squash all the changes in a single commit?

Please add typing on all the methods that you add in this PR, except the tests.

Also, unfortunately I see some of our verbal conversation feedback was not addressed.

Do you want to add typing also in variables or is it overkill?

for example email_or_username: str = data.get("member", "")

[suricactus]

Please add typing on all the methods that you add in this PR, except the tests.

Do you want to add typing also in variables or is it overkill?

for example email_or_username: str = data.get("member", "")

Add typing only when the type cannot be guessed by the type checker. In the case you mentioned, the typechecker has no way to get what is the type at key "member", so it will be Any. So in this case you should add explicit typing that it is str.

But if you have:

def foo() -> int: ...
bar = foo()

There is no need to set the type of bar, as we already set that foo is always returning an int.

[suricactus]

Suggested change

	email_or_username = data.get("member")
	email_or_username: str = data["member"]

We can safely assume the value will always be present in the dict, as the Serializer will automatically check for it. I might be wrong, to be verified.

[tdrivas]

I will check it!

[suricactus]

Are you going to check it :P

[duke-nyuki]

Task linked: QF-4536 Add API endpoint to manage teams

[suricactus]

Good! We are ready to merge this PR. One last renaming and we are good to merge!

[suricactus]

We are there! Thanks a lot!

Please squash all the changes into a single commit and merge the PR.