diff --git a/CHANGELOG.md b/CHANGELOG.md index 1652aad9..77164813 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable user-facing changes to this project will be documented in this file. ## Unreleased +- Finishing the Creator or Builder journey now actually grants the role: since late June the final "Become a Creator" / "Claim Builder Role" step failed for every new member with a generic error, and completion errors now show their real reason instead of a dead-end "try again" (9d546e70) + - Validators can link Telegram support groups to their validator: generate a one-time code on the new Telegram Support page, paste it in a Telegram group with the Deckard support bot, and the group is bound to the validator (multiple groups supported, codes expire in 48 hours and can be revoked) (0cd7e5f) - Marketing can create campaign links like portal.genlayer.foundation/join/builders/ethcc from the admin panel without a deployment; each link tracks visits, signups, and role activations per campaign, campaign traffic reaches Google Analytics with clean final URLs, and new accounts are attributed to the campaign that brought them (810bdc32) diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 70f04f3f..bc5b2baa 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -682,6 +682,18 @@ The project uses **context-aware serialization** to optimize API performance: - `LightEvidenceURLTypeSerializer` - Minimal (id, name, slug, is_generic) for nested use in Evidence responses - `EvidenceURLTypeSerializer` - Full serializer with url_patterns for client-side detection, used in ContributionType responses +### request.user is a lazy wrapper (never `type(request.user)`) + +`EthereumAuthentication` returns Django's `request._request.user`, which is a +`SimpleLazyObject`. Attribute access proxies to the real User (`.pk`, `hasattr`, +serialization all work, and tests using `force_authenticate` pass because they +inject a concrete User), but `type(request.user)` returns the wrapper class, +which has no `.objects` manager. This broke every new Creator/Builder role grant +in production for five weeks while all tests stayed green. Use +`get_user_model()` for manager access, and cover session-authenticated view +paths with `ethereum_auth.testing.login_wallet_session` (guards: +`test_*_through_wallet_session_auth` in `users/tests/test_*_journey.py`). + ### Per-request user lookup (do not reintroduce the address scan) `EthereumAuthentication` (`ethereum_auth/authentication.py`) runs on EVERY DRF request: diff --git a/backend/tally/middleware/logging_utils.py b/backend/tally/middleware/logging_utils.py index 5e0b5959..1a0ab489 100644 --- a/backend/tally/middleware/logging_utils.py +++ b/backend/tally/middleware/logging_utils.py @@ -75,7 +75,12 @@ def format(self, record: logging.LogRecord) -> str: message = record.getMessage() level = record.levelname - return f"[{layer}] {level}: {message}{cid_suffix}" + formatted = f"[{layer}] {level}: {message}{cid_suffix}" + # logger.exception callers rely on the traceback reaching the output; + # the JSON formatter renders it, so the dev console must too. + if record.exc_info: + formatted = f"{formatted}\n{self.formatException(record.exc_info)}" + return formatted class LayeredJSONFormatter(logging.Formatter): diff --git a/backend/tally/tests/test_logging_formatters.py b/backend/tally/tests/test_logging_formatters.py new file mode 100644 index 00000000..01053e30 --- /dev/null +++ b/backend/tally/tests/test_logging_formatters.py @@ -0,0 +1,35 @@ +import logging +import sys + +from django.test import SimpleTestCase + +from tally.middleware.logging_utils import LayeredFormatter, LayeredJSONFormatter + + +def _record_with_exception(): + try: + raise RuntimeError('formatter boom') + except RuntimeError: + exc_info = sys.exc_info() + return logging.LogRecord( + name='tally.app.users', level=logging.ERROR, pathname=__file__, + lineno=1, msg='Failed to complete community journey', args=(), + exc_info=exc_info, + ) + + +class LayeredFormatterExceptionTest(SimpleTestCase): + """Both formatters must render exc_info: logger.exception call sites are + the only durable traceback source once a view catches the error (catching + suppresses django.request's own 500 logging).""" + + def test_console_formatter_appends_traceback(self): + output = LayeredFormatter().format(_record_with_exception()) + self.assertIn('[APP] ERROR: Failed to complete community journey', output) + self.assertIn('Traceback', output) + self.assertIn('formatter boom', output) + + def test_json_formatter_includes_traceback(self): + output = LayeredJSONFormatter().format(_record_with_exception()) + self.assertIn('Traceback', output) + self.assertIn('formatter boom', output) diff --git a/backend/users/tests/test_builder_journey.py b/backend/users/tests/test_builder_journey.py index 518990bc..76999de6 100644 --- a/backend/users/tests/test_builder_journey.py +++ b/backend/users/tests/test_builder_journey.py @@ -178,6 +178,22 @@ def test_complete_grants_role_point_free(self): LeaderboardEntry.objects.filter(user=self.user, type='builder').exists() ) + def test_complete_grants_role_through_wallet_session_auth(self): + # Same grant as test_complete_grants_role_point_free, but through the + # real session authentication chain: EthereumAuthentication returns + # Django's SimpleLazyObject wrapper, which force_authenticate cannot + # model (type(request.user) on the wrapper has no .objects manager). + from ethereum_auth.testing import login_wallet_session + self.client.force_authenticate(user=None) + login_wallet_session(self.client, self.user) + self.complete_star_task() + + response = self.client.post('/api/v1/users/complete_builder_journey/') + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + from builders.models import Builder + self.assertTrue(Builder.objects.filter(user=self.user).exists()) + def test_complete_is_idempotent(self): self.complete_star_task() diff --git a/backend/users/tests/test_community_journey.py b/backend/users/tests/test_community_journey.py index 26c91304..980f2241 100644 --- a/backend/users/tests/test_community_journey.py +++ b/backend/users/tests/test_community_journey.py @@ -281,6 +281,21 @@ def test_complete_grants_creator(self): self.assertEqual(res.status_code, status.HTTP_201_CREATED) self.assertTrue(Creator.objects.filter(user=self.user).exists()) + def test_complete_grants_creator_through_wallet_session_auth(self): + # Production resolves request.user through EthereumAuthentication, which + # returns Django's SimpleLazyObject wrapper. force_authenticate injects + # a concrete User, so it cannot catch wrapper-only breakage such as + # type(request.user) having no .objects manager. + from ethereum_auth.testing import login_wallet_session + self.client.force_authenticate(user=None) + login_wallet_session(self.client, self.user) + self.start_journey() + self.complete_steps_1_to_4() + CommunityPostProof.objects.create(user=self.user, post_url=POST_URL, tweet_id='1790000000000000000') + res = self.client.post('/api/v1/users/complete_community_journey/') + self.assertEqual(res.status_code, status.HTTP_201_CREATED) + self.assertTrue(Creator.objects.filter(user=self.user).exists()) + def test_complete_is_idempotent(self): self.start_journey() self.complete_steps_1_to_4() @@ -291,6 +306,34 @@ def test_complete_is_idempotent(self): self.assertEqual(second.status_code, status.HTTP_200_OK) self.assertEqual(Creator.objects.filter(user=self.user).count(), 1) + def test_complete_failure_returns_json_error_and_rolls_back(self): + # An unhandled exception used to produce an HTML 500 the portal can only + # render as a generic "try again"; it must come back as JSON with the + # real reason, be logged with a traceback, and roll back the role grant. + self.start_journey() + self.complete_steps_1_to_4() + CommunityPostProof.objects.create(user=self.user, post_url=POST_URL, tweet_id='1790000000000000000') + # Patched around the POST only: contribution signals in the setup steps + # call the same function. + with ( + patch('leaderboard.models.update_user_leaderboard_entries', + side_effect=RuntimeError('leaderboard exploded')), + self.assertLogs('tally.app.users', level='ERROR') as logs, + ): + res = self.client.post('/api/v1/users/complete_community_journey/') + # The log must carry the traceback, not just a message: a bare ERROR + # record would pass assertLogs while losing the diagnostic value. + logged = '\n'.join(logs.output) + self.assertIn('Traceback', logged) + self.assertIn('leaderboard exploded', logged) + self.assertEqual(res.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + # The real reason belongs in the logs only: exception text can carry + # table/constraint names or row values, and the portal shows these + # response fields verbatim. + self.assertEqual(res.data['error'], 'completion_failed') + self.assertNotIn('leaderboard exploded', str(res.data)) + self.assertFalse(Creator.objects.filter(user=self.user).exists()) + def test_existing_creator_is_grandfathered(self): # A pre-existing creator who never went through the # new journey is grandfathered in: treated as a complete member and diff --git a/backend/users/views.py b/backend/users/views.py index fe7a3bb3..b0b7fc27 100644 --- a/backend/users/views.py +++ b/backend/users/views.py @@ -596,8 +596,10 @@ def complete_builder_journey(self, request): # Recalculate leaderboard entries now that the Builder relation # exists. The grant itself adds no points, but builder-category # aggregation keys off the Builder profile being present. + # get_user_model(), never type(user): request.user is Django's + # SimpleLazyObject wrapper, whose type has no .objects manager. from leaderboard.models import update_user_leaderboard_entries - fresh_user = type(user).objects.get(pk=user.pk) + fresh_user = get_user_model().objects.get(pk=user.pk) update_user_leaderboard_entries(fresh_user) serializer = self.get_serializer(user) @@ -606,10 +608,17 @@ def complete_builder_journey(self, request): 'user': serializer.data }, status=status.HTTP_201_CREATED) - except Exception as e: - logger.error(f"Failed to complete builder journey: {str(e)}") + except Exception: + # logger.exception keeps the traceback: catching the error here + # suppresses django.request's own 500 logging. The client gets a + # stable message only; str(e) can leak table/constraint names and + # row values, and the portal renders these fields verbatim. + logger.exception('Failed to complete builder journey') return Response( - {'error': f'Failed to complete journey: {str(e)}'}, + { + 'error': 'completion_failed', + 'message': 'Something went wrong on our side while completing the journey. Please try again in a moment.', + }, status=status.HTTP_500_INTERNAL_SERVER_ERROR ) @@ -869,25 +878,43 @@ def complete_community_journey(self, request): from django.db import transaction from django.contrib.auth import get_user_model - with transaction.atomic(): - # Lock the user row so two concurrent requests can't both pass the - # hasattr check above and race into a OneToOne IntegrityError. - get_user_model().objects.select_for_update().get(pk=user.pk) - _, created = Creator.objects.get_or_create(user=user) + try: + with transaction.atomic(): + # Lock the user row so two concurrent requests can't both pass the + # hasattr check above and race into a OneToOne IntegrityError. + get_user_model().objects.select_for_update().get(pk=user.pk) + _, created = Creator.objects.get_or_create(user=user) - if not created: - return Response( - {'message': 'You are already a creator', 'user': self.get_serializer(user).data}, - status=status.HTTP_200_OK, - ) + if not created: + return Response( + {'message': 'You are already a creator', 'user': self.get_serializer(user).data}, + status=status.HTTP_200_OK, + ) - fresh_user = type(user).objects.get(pk=user.pk) - update_user_leaderboard_entries(fresh_user) + # get_user_model(), never type(user): request.user is Django's + # SimpleLazyObject wrapper, whose type has no .objects manager. + fresh_user = get_user_model().objects.get(pk=user.pk) + update_user_leaderboard_entries(fresh_user) - return Response( - {'message': 'Welcome to the GenLayer community!', 'user': self.get_serializer(fresh_user).data}, - status=status.HTTP_201_CREATED, - ) + return Response( + {'message': 'Welcome to the GenLayer community!', 'user': self.get_serializer(fresh_user).data}, + status=status.HTTP_201_CREATED, + ) + except Exception: + # An unhandled error here surfaces as an HTML 500 the portal can + # only render as a generic "try again" dead end. Catching it drops + # django.request's traceback logging, so log it ourselves. The + # client gets a stable message only; str(e) can leak table or + # constraint names and row values, and the portal renders these + # fields verbatim. + logger.exception('Failed to complete community journey') + return Response( + { + 'error': 'completion_failed', + 'message': 'Something went wrong on our side while completing the journey. Please try again in a moment.', + }, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) @action(detail=False, methods=['get']) def validators(self, request):