Conversation
There was a problem hiding this comment.
Pull request overview
This PR starts a major refactor/migration of the server codebase from the legacy FastAPI/SQLModel implementation to a Django + Django REST Framework API, along with updates to documentation, Docker templates, and inclusion of OMOP-related models/utilities.
Changes:
- Added/updated Django project skeleton (
citizens_project/) and DRF authentication endpoints (Google login + JWT). - Removed large portions of the legacy FastAPI/SQLModel server code and related docs/docker artifacts under
old/src/old/docs/old/docker. - Added/updated assorted documentation and ML/data scripts under
docs/andold/(OMOP mappings, model scripts, dataset utilities).
Reviewed changes
Copilot reviewed 80 out of 141 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | New dependency list for Django-based stack (currently malformed encoding). |
| .gitignore | Simplified ignore rules (now risks committing env/secret files). |
| .env.model | Updated env template for DB + Google credentials (formatting issues). |
| .dockerignore | Updated Docker build ignore list. |
| docker/dockerfile | Dockerfile updated for Django runserver. |
| docker/docker-compose-model.yml | New docker-compose template for Postgres + Django web. |
| citizens_project/citizens_project/settings.py | Django settings updated for Postgres + DRF + Google OAuth. |
| citizens_project/citizens_project/urls.py | Project URL routing updated (API mounted under /api/). |
| citizens_project/citizens_project/asgi.py | Django ASGI config doc link/version update. |
| citizens_project/citizens_project/wsgi.py | Django WSGI config doc link/version update. |
| citizens_project/app_cicatrizando/api.py | DRF ViewSets for auth/login/google and auth/me. |
| citizens_project/app_cicatrizando/urls.py | DRF router wiring + Swagger UI route. |
| citizens_project/app_cicatrizando/serializers.py | Serializers adjusted to fields='__all__'. |
| citizens_project/app_cicatrizando/apps.py | AppConfig simplified. |
| citizens_project/app_cicatrizando/google.py | Minor formatting change (Google helper module). |
| citizens_project/app_cicatrizando/views.py | Replaced prior viewsets with placeholder view module. |
| citizens_project/app_cicatrizando/tests.py | Added placeholder Django test module. |
| citizens_project/app_cicatrizando/omop/omop_models.py | Minor whitespace cleanup. |
| citizens_project/app_cicatrizando/omop/omop_serializers.py | Minor formatting cleanup. |
| citizens_project/app_cicatrizando/omop/omop_ids.py | Minor formatting cleanup. |
| citizens_project/app_cicatrizando/admin.py | Removed prior admin registrations; placeholder remains. |
| docs/Readme.md | New Portuguese setup/run documentation. |
| docs/Descricao1.txt | Minor formatting update. |
| docs/model.md | Placeholder (empty). |
| docs/mapeamento_conceptID/cid.py | Cleanup of inline markers/typos in mapping constants/comments. |
| docs/mapeamento_conceptID/wound_type.txt | Minor formatting cleanup. |
| docs/mapeamento_conceptID/smoke_frequency.txt | Formatting fix for closing brace indentation. |
| docs/mapeamento_conceptID/Descricao1.txt | Added mapping documentation draft. |
| docs/mapeamento_conceptID/Descricao2.txt | Added extended OMOP mapping description draft. |
| docs/mapeamento_conceptID/Descricao3.txt | Added additional mapping/notes draft. |
| docs/identification_model/* | Added model training/inference utilities + README + gitignore. |
| docs/infection_ischemia_model/* | Added multi-label training/inference utilities + gitignore. |
| old/requirements.txt | Replaced legacy dependency set with Django-focused set (contains duplicates). |
| old/README.md | Updated legacy README content to Django-based instructions. |
| old/Dockerfile | Updated legacy Dockerfile from FastAPI to Django runserver. |
| old/.gitignore | Added ignore rules in old/ subtree. |
| old/.env.model | Updated legacy env template fields. |
| old/filter_concepts.py | Added one-off CSV filtering script. |
| old/src/** (multiple) | Removed legacy FastAPI/SQLModel implementation modules (auth/db/api/schema/utils). |
| old/docs/** (multiple) | Removed legacy FastAPI/SQLModel/Postgres docs. |
| old/docker/** (multiple) | Removed legacy docker compose templates and MinIO Dockerfile. |
| old/migrations/** (multiple) | Removed Alembic migration scaffolding. |
| README.md | Deleted root README (docs moved under docs/ and old/). |
| docker-compose.yml | Removed old root-level compose file. |
| docker/dbms/docker-compose-modelo.yml | Removed older DB-only compose template. |
Comments suppressed due to low confidence (1)
docker/dockerfile:2
- Using an unpinned base image (
FROM python) can cause non-reproducible builds and sudden breakages when the default tag updates. Pin to a specific major/minor (and ideally a slim variant) that matches the project's supported runtime.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| urlpatterns = [ | ||
| path('', include(router.urls)), | ||
| path('', include(virtual_urls.router.urls)), | ||
| path("api/schema/", SpectacularAPIView.as_view(), name="schema"), | ||
| path("docs/redoc", SpectacularRedocView.as_view(), name="redoc"), | ||
| path("docs/swagger", SpectacularSwaggerView.as_view(), name="schema-swagger-ui"), | ||
| path("docs", scalar_viewer, name="schema-scalar-ui"), | ||
| ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) No newline at end of file | ||
|
|
||
| path('docs/', SpectacularSwaggerView.as_view(url_name='schema'), name='swagger-ui'), | ||
| ] |
There was a problem hiding this comment.
SpectacularSwaggerView.as_view(url_name='schema') will raise NoReverseMatch because there is no URL pattern named schema registered (no SpectacularAPIView route is defined). Add a schema endpoint (and name it schema) or update url_name to match the actual schema route.
| urlpatterns = [ | ||
| path('api-auth/', include('rest_framework.urls', namespace='rest_framework')), | ||
| path('', include('app_cicatrizando.urls')), | ||
| path('admin/', admin.site.urls), |
There was a problem hiding this comment.
This module imports SpectacularAPIView / SpectacularSwaggerView but does not register a schema route. Since app_cicatrizando/urls.py references url_name='schema', the project URLconf should expose a SpectacularAPIView path named schema (typically under /api/schema/) or remove the unused imports and adjust the docs route accordingly.
| path('admin/', admin.site.urls), | |
| path('admin/', admin.site.urls), | |
| path('api/schema/', SpectacularAPIView.as_view(), name='schema'), | |
| path('api/docs/', SpectacularSwaggerView.as_view(url_name='schema'), name='api-docs'), |
| logger.debug("Google login request") | ||
| auth_serializer = self.serializer_class(data=request.data) | ||
| auth_serializer.is_valid(raise_exception=True) # Valida os dados da requisição. | ||
| auth_serializer.is_valid(raise_exception=True) | ||
| validated_data = auth_serializer.validated_data | ||
| logger.debug(f"Validated data: {validated_data}") | ||
| # Obtém os dados do usuário do Google. | ||
|
|
||
| # Obtém os dados do usuário do Google | ||
| user_data = google_get_user_data(validated_data) | ||
| logger.debug(f"User data from Google: {user_data}") |
There was a problem hiding this comment.
These debug logs include validated_data (OAuth code / ID token) and user_data (PII such as email/name). This can leak sensitive data into logs. Remove these logs or redact sensitive fields before logging (and prefer structured logging with explicit safe fields).
| REST_FRAMEWORK = { | ||
| 'DEFAULT_AUTHENTICATION_CLASSES': [ | ||
| 'rest_framework_simplejwt.authentication.JWTAuthentication', | ||
| ], |
There was a problem hiding this comment.
REST_FRAMEWORK sets authentication but not default permissions, so endpoints without an explicit permission_classes will default to AllowAny. If the intended default is authenticated access (as in the prior code), set DEFAULT_PERMISSION_CLASSES to IsAuthenticated and override only the login endpoints.
| ], | |
| ], | |
| 'DEFAULT_PERMISSION_CLASSES': [ | |
| 'rest_framework.permissions.IsAuthenticated', | |
| ], |
| SERVER_WOUNDS_DB_HOST=db # | ||
| SERVER_WOUNDS_DB_PORT=5432 | ||
| SERVER_WOUNDS_SECRET_KEY="chave" | ||
| SERVER_WOUNDS_DISABLE_AUTH=false No newline at end of file | ||
|
|
||
| SERVER_WOUNDS_SECRET_KEY= your-secret-key-here # | ||
| GOOGLE_CLIENT_ID= your-client-id # | ||
| GOOGLE_CLIENT_SECRET= your-client-secret # |
There was a problem hiding this comment.
This env template includes leading spaces after = and inline # markers, which many dotenv parsers treat as part of the value (or can be confusing). Remove the leading spaces and move comments to separate lines so copy/paste produces a valid .env.
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| restart: on-failure:3 |
There was a problem hiding this comment.
depends_on: condition: service_healthy requires the db service to define a healthcheck. As written, docker compose will error because db has no healthcheck. Add a Postgres healthcheck (e.g., pg_isready) or change the dependency condition to service_started.
| # SECURITY WARNING: keep the secret key used in production secret! | ||
| SECRET_KEY = os.environ["SERVER_WOUNDS_SECRET_KEY"] | ||
| GOOGLE_OAUTH2_CLIENT_ID = os.environ["GOOGLE_CLIENT_ID"] | ||
| GOOGLE_OAUTH2_CLIENT_SECRET = os.environ["GOOGLE_CLIENT_SECRET"] | ||
| DISABLE_AUTH = "true" == os.environ.get("SERVER_WOUNDS_DISABLE_AUTH", "false") | ||
| SECRET_KEY = os.environ.get("SERVER_WOUNDS_SECRET_KEY", "django-insecure-changeme") | ||
|
|
||
| # SECURITY WARNING: don't run with debug turned on in production! | ||
| DEBUG = True | ||
|
|
||
| ALLOWED_HOSTS = [ | ||
| 'server.wounds.paas.ic.unicamp.br', | ||
| 'server.wounds.staging.paas.ic.unicamp.br', | ||
| 'localhost', | ||
| '127.0.0.1' | ||
| ] | ||
| ALLOWED_HOSTS = [] | ||
|
|
There was a problem hiding this comment.
SECRET_KEY falls back to a hardcoded insecure default, and ALLOWED_HOSTS is always empty (even though docker-compose provides ALLOWED_HOSTS). This makes it easy to misdeploy with an unsafe key and can break requests when DEBUG is off. Prefer reading ALLOWED_HOSTS from env (comma-separated) and require SERVER_WOUNDS_SECRET_KEY when DEBUG is false.
inicio do refactor da parte da API e documentação (modifiquei apenas o readme por enquanto), models usados da versão anterior. google auth implementado.