Skip to content
Open
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
6 changes: 3 additions & 3 deletions .github/workflows/django-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
python-version: 3.11

- name: cache poetry install
uses: actions/cache@v2
uses: actions/cache@v4
with:
path: ~/.local
key: poetry-1.2.2-0
Expand All @@ -32,7 +32,7 @@ jobs:

- name: cache deps
id: cache-deps
uses: actions/cache@v2
uses: actions/cache@v4
with:
path: .venv
key: pydeps-${{ hashFiles('**/poetry.lock') }}
Expand All @@ -46,4 +46,4 @@ jobs:
- name: Run tests
run: poetry run python manage.py test
env:
DEBUG: True
DEBUG: True
2 changes: 2 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ ENV PYTHONFAULTHANDLER=1 \
POETRY_VERSION=1.2.2

RUN pip install "poetry==$POETRY_VERSION"
RUN apt install -y cmake
RUN rm -rf /var/lib/apt/lists/*

RUN mkdir /procollab

Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ services:
command: bash ./scripts/startup.sh
volumes:
- ./log:/procollab/log
- ./:/procollab/.
env_file:
- .env
environment:
Expand Down
44 changes: 44 additions & 0 deletions users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,31 @@ class UserList(ListCreateAPIView):
filter_backends = (filters.DjangoFilterBackend,)
filterset_class = UserFilter

def list(self, request, *args, **kwargs):
max_skills = request.query_params.get("max_skills", None)
Copy link
Member

Choose a reason for hiding this comment

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

это нужно прокинуть в контекст сериалайзера, чтобы мы из БД тянули скиллы с лимитом, а не вытаскивали ВСЕ и потом обрезали вывод

Copy link
Member

Choose a reason for hiding this comment

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

давай тут вместо None какой-нибудь разумный дефолт бахнем? сейчас фронт все равно не передает этот квери параметр и после мержа этого ПРа он продолжит забирать кучу лишних данных, а с дефолтом мы этот размер ограничим

if max_skills is not None:
Copy link
Member

Choose a reason for hiding this comment

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

валидацию квери параметров может сделать через сериалайзер? то, что вьюшка занимается валидацией квери параметров мне кажется не ок

try:
max_skills = int(max_skills)
if max_skills < 0:
return Response(
{"error": "max_skills must be a positive integer"},
Copy link
Member

Choose a reason for hiding this comment

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

да зачем тут ошибку отдавать, давай просто считать что max_skills = 0 и не будем вообще эти скиллы получать из бд

status=status.HTTP_400_BAD_REQUEST,
)
except ValueError:
return Response(
{"error": "max_skills must be an integer"},
status=status.HTTP_400_BAD_REQUEST,
)

response = super().list(request, *args, **kwargs)

if max_skills is not None:
for user_data in response.data.get("results", []):
if "skills" in user_data:
user_data["skills"] = user_data["skills"][:max_skills]

Comment on lines +116 to +119
Copy link
Member

Choose a reason for hiding this comment

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

это должно быть учтено при походе в БД, точно не стоит обрезать кусок ответа во вьюшке)

return response

def post(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
Expand Down Expand Up @@ -259,8 +284,27 @@ class CurrentUser(GenericAPIView):

def get(self, request):
user = request.user

max_skills = request.query_params.get("max_skills", None)
if max_skills is not None:
try:
max_skills = int(max_skills)
if max_skills < 0:
return Response(
{"error": "max_skills must be a positive integer"},
status=status.HTTP_400_BAD_REQUEST,
)
except ValueError:
return Response(
{"error": "max_skills must be an integer"},
status=status.HTTP_400_BAD_REQUEST,
)

Comment on lines +288 to +302
Copy link
Member

Choose a reason for hiding this comment

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

лютая копипаста, точно нужно убрать это отсюда

serializer = self.get_serializer(user)

if max_skills is not None and "skills" in serializer.data:
serializer.data["skills"] = serializer.data["skills"][:max_skills]

if settings.DEBUG:
skills_url_name = (
"https://skills.dev.procollab.ru/progress/subscription-data/"
Expand Down