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

[WIP] Add Teams Endpoints #1031

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open

Conversation

tdrivas
Copy link

@tdrivas tdrivas commented Sep 17, 2024

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

@suricactus suricactus changed the title Add Teams Endpoints [WIP] Add Teams Endpoints Oct 15, 2024
@tdrivas
Copy link
Author

tdrivas commented Oct 30, 2024

Planned Tasks

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

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

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

.gitignore Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/serializers.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-app/qfieldcloud/core/views/users_views.py Outdated Show resolved Hide resolved
docker-compose.override.local.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

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

@dddpt dddpt self-requested a review November 26, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants