From 048dfef1b54731193f43fb850851c218cb25cd0b Mon Sep 17 00:00:00 2001 From: JoaquinBN Date: Thu, 6 Aug 2026 13:02:26 +0200 Subject: [PATCH 1/4] Add stored submission resubmission flows --- backend/CLAUDE.md | 15 + .../0085_submissionmoreinforesponse.py | 80 +++ backend/contributions/models.py | 52 ++ backend/contributions/serializers.py | 146 ++++- backend/contributions/tests/test_appeal.py | 17 +- .../tests/test_is_submittable.py | 17 +- .../tests/test_more_info_responses.py | 555 ++++++++++++++++++ .../tests/test_state_transitions.py | 15 +- .../tests/test_validator_category_gating.py | 43 +- backend/contributions/views.py | 323 +++++++--- backend/notifications/services.py | 4 +- backend/notifications/tests.py | 15 +- backend/submissions_review.md | 10 + frontend/CLAUDE.md | 9 +- .../components/StewardSubmissionCard.svelte | 16 + frontend/src/components/SubmissionCard.svelte | 154 ++++- .../SubmitContribution.svelte | 328 ++++++++++- frontend/src/lib/api.js | 25 + frontend/src/routes/MySubmissions.svelte | 30 + frontend/src/routes/SubmitContribution.svelte | 123 +++- frontend/src/tests/EditSubmission.test.js | 49 +- .../src/tests/StewardSubmissionCard.test.js | 24 + frontend/src/tests/SubmissionCard.test.js | 118 ++++ .../tests/SubmitContributionResubmit.test.js | 399 +++++++++++++ 24 files changed, 2426 insertions(+), 141 deletions(-) create mode 100644 backend/contributions/migrations/0085_submissionmoreinforesponse.py create mode 100644 backend/contributions/tests/test_more_info_responses.py create mode 100644 frontend/src/tests/SubmitContributionResubmit.test.js diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index bc5b2baa..75169183 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -73,6 +73,7 @@ backend/ - ContributionType - Categories with slug field, M2M `accepted_evidence_url_types`, optional global lifetime `max_submissions`, and optional `max_submissions_per_user_per_week`. The weekly limit uses Monday-Sunday UTC `SubmittedContribution.created_at` bounds and counts every state; edits/appeals reuse the same row and do not consume another slot. If an editable submission changes contribution type, the target type's capacity is checked in the submission's original creation week. `requires_ai_review` gates unengaged pending submissions from tier-1 stewards, while `escalation_threshold_points` converts tier-1 accepts whose contribution-date multiplied points meet the threshold into proposals. Migration 0084 backfills Builder-category types once; new Builder types default to `True` / `400` for values not supplied explicitly, and later explicit admin/model updates remain unchanged. - AIReviewFeedback - Per-reviewer, per-AI-proposal benchmark feedback with an immutable `(proposal_source, proposal_source_id)` binding, timestamp metadata in `proposal_ref`, verdict, optional corrected decision/rubric ranges, typed anchored error claims, and a best-effort commit SHA pinned on first save. Records are unique by `(submitted_contribution, reviewer, proposal_source, proposal_source_id)` and never alter submission review state. - SubmissionStateTransition - Append-only lifecycle log for submissions (migration 0079). One row per event (`submitted`/`review`/`bulk_reject`/`gate_reject`/`edited`/`canceled`/`appeal`/`evidence_added`/`escalated`/`admin`) with from_state/to_state/actor. Written by every path that changes `state` or clears `reviewed_by`/`reviewed_at` (creation via post_save signal; the rest inline at each call site, incl. the Tier-1 gate command and admin `save_model`). Never mutate or delete rows; read-only in admin. Rationale: row state is overwritten in place and re-open paths destroy review fields, so this log is the only durable decision/lifecycle history. Bulk reject also writes a per-submission decision SubmissionNote (`data.action='reject'`, `data.bulk=true`) so bulk decisions appear in the CRM timeline and note-based metrics like single rejects. The dead `resubmitted_more_info` filter (relied on `reviewed_at` surviving edits, impossible since 2026-06-22) was removed from both filtersets and the steward search grammar. + - SubmissionMoreInfoResponse - Immutable submitter response paired one-to-one with a structured request-more-info `SubmissionNote`, plus a nullable request-note FK for legacy `staff_reply` requests. It snapshots the request text/author/time, responder, and capped 1,000-character response so every request cycle remains auditable after the submission returns to pending. Submitter and steward serializers expose it as `more_info_requests[].response`; legacy requests expose `response: null` until actually answered and never receive fabricated history. - Human review hierarchy - `Steward.tier` is reviewer (1), top-level (2), or apex (3); steward superusers act as tier 3. Tier 2+ gets the full permission matrix and unrestricted submission visibility. `SubmittedContribution.escalated_at` marks an active threshold proposal and is exposed only through the steward serializer. Tier-1 AI-gated visibility accepts durable AI proposal notes/rows, any active proposal, appeals, and transition-backed more-info resubmissions. Search supports `is_escalated`; the portal grammar maps `is:escalated` / `not:escalated`. - Reviewer reward economy - Direct tier-1 decisions on threshold-enabled types earn 10 points for reviewing another user's submission; dedup uses the exact notes key `Review decision reward for submission {submission.id} [{action}]`, while bulk rejects, escalated accepts, and tier 2+ decisions are excluded. Every escalation records a human `ReviewProposal`, so a different finalizer rewards the escalator using rubric agreement for Builder Projects or binary action agreement for standard flows. All human-proposal rewards are then reduced proportionally when final points differ from proposed points. - **Projects/Milestones split**: `contributions/project_milestones.py` @@ -478,10 +479,24 @@ DELETE /api/v1/contributions/{id}/ (requires auth) # Submissions (submitter-side) GET /api/v1/submissions/my/ (requires auth, paginated user submissions) +GET /api/v1/submissions/{id}/ (requires auth, owner-only source/detail lookup) +PUT /api/v1/submissions/{id}/ (requires auth, owner-only editable submission update) +PATCH /api/v1/submissions/{id}/ (requires auth, owner-only editable submission update) GET /api/v1/submissions/accepted-projects/ (requires auth, the user's highlighted Projects contributions milestones can link to; optional ?submission=UUID includes that pending milestone's grandfathered current link; includes next_milestone_version and github_url) POST /api/v1/submissions/{id}/appeal/ (requires auth, owner-only, one per submission) POST /api/v1/submissions/{id}/add-evidence/ (requires auth, owner-only) +# More-info resubmission contract +# A PUT/PATCH while state=more_info_needed must include this write-only object: +# "more_info_response": {"request_id": 123, "message": "What changed"} +# request_id is null only for the current legacy staff_reply fallback. The +# server locks the row, revalidates the resulting snapshot with current type, +# mission, role/social/Discord, capacity and evidence rules, writes the paired +# response, clears review fields, and transitions the same row to pending in +# one transaction. Blank, stale, mismatched, duplicate and pending-state +# responses are rejected. Read payloads expose the result under +# more_info_requests[].response as {id, message, user, user_name, created_at}. + # Contribution Types GET /api/v1/contribution-types/ (requires auth) GET /api/v1/contribution-types/{id}/ (requires auth) diff --git a/backend/contributions/migrations/0085_submissionmoreinforesponse.py b/backend/contributions/migrations/0085_submissionmoreinforesponse.py new file mode 100644 index 00000000..32d993e1 --- /dev/null +++ b/backend/contributions/migrations/0085_submissionmoreinforesponse.py @@ -0,0 +1,80 @@ +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contributions', '0084_enable_builder_review_hierarchy'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='SubmissionMoreInfoResponse', + fields=[ + ( + 'id', + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('request_message', models.TextField()), + ('requested_at', models.DateTimeField(blank=True, null=True)), + ('message', models.TextField(max_length=1000)), + ( + 'request_note', + models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='submitter_response', + to='contributions.submissionnote', + ), + ), + ( + 'requested_by', + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='more_info_requests_answered', + to=settings.AUTH_USER_MODEL, + ), + ), + ( + 'responder', + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='submission_more_info_responses', + to=settings.AUTH_USER_MODEL, + ), + ), + ( + 'submitted_contribution', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='more_info_responses', + to='contributions.submittedcontribution', + ), + ), + ], + options={ + 'ordering': ['-created_at', '-id'], + 'indexes': [ + models.Index( + fields=['submitted_contribution', 'created_at'], + name='sub_info_resp_created_idx', + ), + ], + }, + ), + ] diff --git a/backend/contributions/models.py b/backend/contributions/models.py index 956c6378..d5c75d74 100644 --- a/backend/contributions/models.py +++ b/backend/contributions/models.py @@ -1096,6 +1096,58 @@ def __str__(self): return f"{prefix}{self.user} on {self.submitted_contribution} at {self.created_at}" +class SubmissionMoreInfoResponse(BaseModel): + """A submitter's durable response to a steward more-information request. + + New responses point at the structured ``SubmissionNote`` that recorded the + request. ``request_note`` remains nullable so legacy submissions whose + request only survives in ``staff_reply`` can still be answered; the request + snapshot fields preserve that context after ``staff_reply`` is cleared. + """ + + submitted_contribution = models.ForeignKey( + SubmittedContribution, + on_delete=models.CASCADE, + related_name='more_info_responses', + ) + request_note = models.OneToOneField( + SubmissionNote, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name='submitter_response', + ) + request_message = models.TextField() + requested_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name='more_info_requests_answered', + ) + requested_at = models.DateTimeField(null=True, blank=True) + responder = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name='submission_more_info_responses', + ) + message = models.TextField(max_length=1000) + + class Meta: + ordering = ['-created_at', '-id'] + indexes = [ + models.Index( + fields=['submitted_contribution', 'created_at'], + name='sub_info_resp_created_idx', + ), + ] + + def __str__(self): + return f"Response on {self.submitted_contribution_id} at {self.created_at}" + + class SubmissionStateTransition(BaseModel): """ Append-only log of submission lifecycle events. diff --git a/backend/contributions/serializers.py b/backend/contributions/serializers.py index c200cf28..b719b18a 100644 --- a/backend/contributions/serializers.py +++ b/backend/contributions/serializers.py @@ -7,7 +7,7 @@ ContributionHighlight, Mission, StartupRequest, SubmissionNote, FeaturedContent, Alert, EvidenceURLType, ContributionDiscordXPState, DiscordXPDistributionEvent, ProjectMilestoneReview, ReviewProposal, - AIReviewFeedback, + AIReviewFeedback, SubmissionMoreInfoResponse, ) from .ai_attribution import AI_STEWARD_EMAIL from .ai_feedback import normalize_feedback_payload @@ -560,18 +560,70 @@ class Meta: read_only_fields = ['url_type', 'created_at'] +class MoreInfoResponseInputSerializer(serializers.Serializer): + """Write-only response supplied when reopening a more-info submission.""" + + request_id = serializers.IntegerField(required=True, allow_null=True) + message = serializers.CharField( + required=True, + allow_blank=False, + max_length=1000, + trim_whitespace=True, + ) + + class MoreInfoRequestsMixin(serializers.Serializer): more_info_requests = serializers.SerializerMethodField() + @staticmethod + def _user_name(user): + if not user: + return None + return user.name or ( + f'{user.address[:10]}...' if user.address else str(user.id) + ) + + def _serialize_more_info_response(self, response): + if not response: + return None + return { + 'id': response.id, + 'message': response.message, + 'user': response.responder_id, + 'user_name': self._user_name(response.responder), + 'created_at': ( + response.created_at.isoformat() + if response.created_at else None + ), + } + def get_more_info_requests(self, obj): notes = getattr(obj, 'more_info_request_notes', None) if notes is None: - notes = ( + notes = list( obj.internal_notes .filter(is_proposal=False, data__action='more_info') .select_related('user') .order_by('-created_at', '-id') ) + else: + notes = list(notes) + + responses = getattr(obj, 'more_info_response_rows', None) + if responses is None: + responses = list( + obj.more_info_responses + .select_related('request_note', 'requested_by', 'responder') + .order_by('-created_at', '-id') + ) + else: + responses = list(responses) + + responses_by_request = { + response.request_note_id: response + for response in responses + if response.request_note_id + } requests = [] for note in notes: @@ -580,14 +632,68 @@ def get_more_info_requests(self, obj): if not message: continue user = note.user - user_name = user.name or (f'{user.address[:10]}...' if user.address else str(user.id)) requests.append({ 'id': note.id, 'message': message, 'user': user.id, - 'user_name': user_name, + 'user_name': self._user_name(user), 'created_at': note.created_at.isoformat() if note.created_at else None, + 'response': self._serialize_more_info_response( + responses_by_request.get(note.id) + ), + 'legacy': False, + '_sort_at': note.created_at.timestamp() if note.created_at else 0, }) + + # Responses to pre-SubmissionNote requests retain an immutable request + # snapshot so the exchange remains visible after staff_reply is cleared. + for response in responses: + if response.request_note_id: + continue + requests.append({ + 'id': None, + 'message': response.request_message, + 'user': response.requested_by_id, + 'user_name': self._user_name(response.requested_by), + 'created_at': ( + response.requested_at.isoformat() + if response.requested_at else None + ), + 'response': self._serialize_more_info_response(response), + 'legacy': True, + '_sort_at': ( + response.requested_at.timestamp() + if response.requested_at else 0 + ), + }) + + has_unanswered_structured_request = any( + request['id'] is not None and request['response'] is None + for request in requests + ) + if ( + obj.state == 'more_info_needed' + and obj.staff_reply + and not has_unanswered_structured_request + ): + requests.append({ + 'id': None, + 'message': obj.staff_reply, + 'user': obj.reviewed_by_id, + 'user_name': self._user_name(obj.reviewed_by), + 'created_at': ( + obj.reviewed_at.isoformat() if obj.reviewed_at else None + ), + 'response': None, + 'legacy': True, + '_sort_at': ( + obj.reviewed_at.timestamp() if obj.reviewed_at else 0 + ), + }) + + requests.sort(key=lambda request: request['_sort_at'], reverse=True) + for request in requests: + request.pop('_sort_at', None) return requests @@ -612,6 +718,10 @@ class SubmittedContributionSerializer(MoreInfoRequestsMixin, serializers.ModelSe required=False, allow_null=True, ) + more_info_response = MoreInfoResponseInputSerializer( + required=False, + write_only=True, + ) recaptcha = ReCaptchaField(required=False) # Required only on create, handled in validate() class Meta: @@ -622,7 +732,8 @@ class Meta: 'proposed_points', 'converted_contribution', 'contribution', 'mission', 'project_contribution', 'milestone_version', 'has_appeal', 'appeal_reason', 'appealed_at', 'more_info_requests', - 'created_at', 'updated_at', 'last_edited_at', 'recaptcha'] + 'created_at', 'updated_at', 'last_edited_at', 'recaptcha', + 'more_info_response'] read_only_fields = ['id', 'user', 'state', 'staff_reply', 'reviewed_by', 'reviewed_at', 'created_at', 'updated_at', 'last_edited_at', 'proposed_points', 'converted_contribution', @@ -703,6 +814,28 @@ def validate(self, data): # Remove recaptcha from validated data as it's not a model field data.pop('recaptcha', None) + more_info_response = data.get('more_info_response') + if self.instance: + if self.instance.state == 'more_info_needed' and not more_info_response: + raise serializers.ValidationError({ + 'more_info_response': ( + 'Tell the steward what changed before resubmitting.' + ), + }) + if self.instance.state != 'more_info_needed' and more_info_response: + raise serializers.ValidationError({ + 'more_info_response': ( + 'A more-information response is only valid while ' + 'information is requested.' + ), + }) + elif more_info_response: + raise serializers.ValidationError({ + 'more_info_response': ( + 'New submissions cannot include a more-information response.' + ), + }) + return data def _validate_evidence_items(self, evidence_items_data, @@ -929,6 +1062,9 @@ def update(self, instance, validated_data): Evidence items with 'id' are updated, items without 'id' are created, and items not in the list are deleted. """ + # The view persists this audit record atomically with the state change. + validated_data.pop('more_info_response', None) + # Extract evidence items from initial_data (raw request data) # since evidence_items is a SerializerMethodField and not in validated_data evidence_items_data = self.initial_data.get('evidence_items', None) diff --git a/backend/contributions/tests/test_appeal.py b/backend/contributions/tests/test_appeal.py index 88cffcf5..6a035624 100644 --- a/backend/contributions/tests/test_appeal.py +++ b/backend/contributions/tests/test_appeal.py @@ -9,6 +9,7 @@ SubmissionNote, ContributionType, Category, + Evidence, ) User = get_user_model() @@ -267,11 +268,23 @@ def test_appealed_more_info_needed_submission_can_be_patched(self): # Steward moves it to more_info_needed submission.refresh_from_db() submission.state = 'more_info_needed' - submission.save(update_fields=['state']) + submission.staff_reply = 'Please provide more context.' + submission.save(update_fields=['state', 'staff_reply']) + Evidence.objects.create( + submitted_contribution=submission, + description='Original evidence', + url='https://example.com/appeal-more-info-evidence', + ) response = self.client.patch( f'/api/v1/submissions/{submission.id}/', - {'notes': 'Here is more info'}, + { + 'notes': 'Here is more info', + 'more_info_response': { + 'request_id': None, + 'message': 'Added the requested context.', + }, + }, format='json', ) # Lock should NOT apply — should not be 403 from the appeal lock diff --git a/backend/contributions/tests/test_is_submittable.py b/backend/contributions/tests/test_is_submittable.py index 9542283b..221c7971 100644 --- a/backend/contributions/tests/test_is_submittable.py +++ b/backend/contributions/tests/test_is_submittable.py @@ -3,7 +3,7 @@ from django.utils import timezone from rest_framework.test import APIClient from rest_framework import status -from contributions.models import ContributionType, Category, SubmittedContribution +from contributions.models import ContributionType, Category, Evidence, SubmittedContribution from users.models import User @@ -276,9 +276,22 @@ def test_pending_submission_of_retired_type_can_be_edited(self): def test_more_info_needed_submission_of_retired_type_can_be_edited(self): submission = self._make_submission(state='more_info_needed') + submission.staff_reply = 'Please provide more context.' + submission.save(update_fields=['staff_reply']) + Evidence.objects.create( + submitted_contribution=submission, + description='Original evidence', + url='https://example.com/retired-type-evidence', + ) response = self.client.patch( f'/api/v1/submissions/{submission.id}/', - {'notes': 'Here is the requested info'}, + { + 'notes': 'Here is the requested info', + 'more_info_response': { + 'request_id': None, + 'message': 'Added the requested context.', + }, + }, format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/backend/contributions/tests/test_more_info_responses.py b/backend/contributions/tests/test_more_info_responses.py new file mode 100644 index 00000000..8c0d3ab6 --- /dev/null +++ b/backend/contributions/tests/test_more_info_responses.py @@ -0,0 +1,555 @@ +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.db import IntegrityError +from django.test import TestCase +from django.utils import timezone +from rest_framework import status +from rest_framework.test import APIClient + +from contributions.models import ( + Category, + ContributionType, + Evidence, + EvidenceURLType, + SubmissionMoreInfoResponse, + SubmissionNote, + SubmissionStateTransition, + SubmittedContribution, +) +from leaderboard.models import GlobalLeaderboardMultiplier +from notifications.models import Notification +from social_connections.models import DiscordRole +from stewards.models import Steward, StewardPermission + + +User = get_user_model() + + +class MoreInfoResponseAPITest(TestCase): + def setUp(self): + self.category = Category.objects.create( + name='Response Test', + slug='response-test', + description='Response test category', + ) + self.contribution_type = ContributionType.objects.create( + name='Response Test Type', + slug='response-test-type', + description='Response test contribution type', + category=self.category, + min_points=1, + max_points=100, + ) + GlobalLeaderboardMultiplier.objects.create( + contribution_type=self.contribution_type, + multiplier_value=1, + valid_from=timezone.now() - timezone.timedelta(days=1), + ) + self.other_evidence_type, _ = EvidenceURLType.objects.update_or_create( + slug='other', + defaults={ + 'name': 'Other', + 'url_patterns': [], + 'is_generic': True, + 'order': 99, + }, + ) + self.owner = User.objects.create_user( + email='response-owner@test.com', + address='0x1111111111111111111111111111111111111111', + password='pass', + name='Response Owner', + ) + self.other_user = User.objects.create_user( + email='response-other@test.com', + address='0x2222222222222222222222222222222222222222', + password='pass', + ) + self.steward_user = User.objects.create_user( + email='response-steward@test.com', + address='0x3333333333333333333333333333333333333333', + password='pass', + name='Response Steward', + ) + self.steward = Steward.objects.create(user=self.steward_user) + StewardPermission.objects.create( + steward=self.steward, + contribution_type=self.contribution_type, + action='accept', + ) + self.client = APIClient() + self.client.force_authenticate(user=self.owner) + + def _make_more_info_submission( + self, + *, + request_message='Please document the repository setup.', + structured=True, + owner=None, + ): + owner = owner or self.owner + submission = SubmittedContribution.objects.create( + user=owner, + contribution_type=self.contribution_type, + contribution_date=timezone.now(), + title='Repository work', + notes='Original submission notes', + state='more_info_needed', + staff_reply=request_message, + reviewed_by=self.steward_user, + reviewed_at=timezone.now(), + assigned_to=self.steward_user, + gate_reviewed=True, + ) + Evidence.objects.create( + submitted_contribution=submission, + url=f'https://example.com/proof/{submission.id}', + description='Proof', + url_type=self.other_evidence_type, + ) + note = None + if structured: + note = SubmissionNote.objects.create( + submitted_contribution=submission, + user=self.steward_user, + message=f'Reviewed: **more_info**\n\n> {request_message}', + is_proposal=False, + data={ + 'action': 'more_info', + 'staff_reply': request_message, + }, + ) + return submission, note + + def _response_payload(self, note, message='Updated the repository documentation.'): + return { + 'more_info_response': { + 'request_id': note.id if note else None, + 'message': message, + }, + } + + def test_response_is_required_and_blank_or_too_long_is_rejected(self): + submission, note = self._make_more_info_submission() + url = f'/api/v1/submissions/{submission.id}/' + + missing = self.client.patch(url, {'notes': 'Edited'}, format='json') + self.assertEqual(missing.status_code, status.HTTP_400_BAD_REQUEST) + + blank = self.client.patch( + url, + self._response_payload(note, message=' '), + format='json', + ) + self.assertEqual(blank.status_code, status.HTTP_400_BAD_REQUEST) + + too_long = self.client.patch( + url, + self._response_payload(note, message='x' * 1001), + format='json', + ) + self.assertEqual(too_long.status_code, status.HTTP_400_BAD_REQUEST) + + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertEqual(submission.notes, 'Original submission notes') + self.assertFalse( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).exists() + ) + + def test_direct_response_reopens_submission_and_serializes_pair(self): + submission, note = self._make_more_info_submission() + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(note), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + submission.refresh_from_db() + self.assertEqual(submission.state, 'pending') + self.assertEqual(submission.staff_reply, '') + self.assertFalse(submission.gate_reviewed) + self.assertIsNone(submission.reviewed_by) + self.assertIsNone(submission.reviewed_at) + + stored = SubmissionMoreInfoResponse.objects.get( + submitted_contribution=submission, + ) + self.assertEqual(stored.request_note, note) + self.assertEqual(stored.request_message, note.data['staff_reply']) + self.assertEqual(stored.requested_by, self.steward_user) + self.assertEqual(stored.responder, self.owner) + + (request_data,) = response.data['more_info_requests'] + self.assertEqual(request_data['id'], note.id) + self.assertEqual(request_data['message'], note.data['staff_reply']) + self.assertEqual( + request_data['response']['message'], + 'Updated the repository documentation.', + ) + self.assertEqual(request_data['response']['user'], self.owner.id) + self.assertIsNotNone(request_data['response']['created_at']) + + transition = SubmissionStateTransition.objects.get( + submitted_contribution=submission, + event=SubmissionStateTransition.EVENT_EDITED, + ) + self.assertEqual( + (transition.from_state, transition.to_state), + ('more_info_needed', 'pending'), + ) + notification = Notification.objects.get( + recipient=self.steward_user, + event_type='submission.more_info_resubmitted', + ) + self.assertIn('responded to your more-information request', notification.body) + + def test_stale_mismatched_and_duplicate_responses_are_rejected(self): + submission, stale_note = self._make_more_info_submission() + current_note = SubmissionNote.objects.create( + submitted_contribution=submission, + user=self.steward_user, + message='Reviewed: **more_info**\n\n> Send the latest release link.', + data={ + 'action': 'more_info', + 'staff_reply': 'Send the latest release link.', + }, + ) + + stale = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(stale_note), + format='json', + ) + self.assertEqual(stale.status_code, status.HTTP_409_CONFLICT) + + other_submission, other_note = self._make_more_info_submission() + mismatched = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(other_note), + format='json', + ) + self.assertEqual(mismatched.status_code, status.HTTP_409_CONFLICT) + + successful = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(current_note), + format='json', + ) + self.assertEqual(successful.status_code, status.HTTP_200_OK) + + duplicate = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(current_note), + format='json', + ) + self.assertEqual(duplicate.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).count(), + 1, + ) + self.assertEqual(other_submission.state, 'more_info_needed') + + def test_source_and_request_are_owner_scoped(self): + submission, note = self._make_more_info_submission() + self.client.force_authenticate(user=self.other_user) + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(note), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertFalse( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).exists() + ) + + def test_failed_snapshot_validation_rolls_back_edits_and_response(self): + submission, note = self._make_more_info_submission() + github_type, _ = EvidenceURLType.objects.update_or_create( + slug='github-repo', + defaults={ + 'name': 'GitHub Repository', + 'url_patterns': [r'^https?://github\.com/[^/]+/[^/]+/?$'], + 'is_generic': False, + 'order': 1, + 'ownership_social_account': '', + }, + ) + self.contribution_type.required_evidence_url_types.set([github_type]) + payload = self._response_payload(note) + payload['notes'] = 'This edit must roll back.' + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + payload, + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertEqual(submission.notes, 'Original submission notes') + self.assertFalse( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).exists() + ) + + @patch( + 'contributions.views.SubmissionMoreInfoResponse.objects.create', + side_effect=IntegrityError('simulated response write failure'), + ) + def test_response_write_failure_rolls_back_submission_edits(self, _create): + submission, note = self._make_more_info_submission() + self.client.raise_request_exception = False + payload = self._response_payload(note) + payload['notes'] = 'This must be rolled back with the response.' + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + payload, + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertEqual(submission.notes, 'Original submission notes') + self.assertEqual(submission.staff_reply, note.data['staff_reply']) + self.assertFalse(submission.more_info_responses.exists()) + + def test_current_social_requirement_blocks_direct_resubmission(self): + submission, note = self._make_more_info_submission() + self.contribution_type.required_social_accounts = ['github'] + self.contribution_type.save(update_fields=['required_social_accounts']) + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(note), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertFalse(submission.more_info_responses.exists()) + + def test_current_discord_role_requirement_blocks_direct_resubmission(self): + submission, note = self._make_more_info_submission() + role = DiscordRole.objects.create( + guild_id='response-guild', + role_id='response-role', + name='Response reviewer', + position=1, + ) + self.contribution_type.required_discord_roles.add(role) + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(note), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertFalse(submission.more_info_responses.exists()) + + def test_current_url_ownership_rule_blocks_direct_resubmission(self): + submission, note = self._make_more_info_submission() + github_type, _ = EvidenceURLType.objects.update_or_create( + slug='github-repo', + defaults={ + 'name': 'GitHub Repository', + 'url_patterns': [r'^https?://github\.com/[^/]+/[^/]+/?$'], + 'handle_extract_pattern': r'github\.com/(?P[^/]+)/', + 'ownership_social_account': 'github', + 'allow_duplicate': False, + 'is_generic': False, + 'order': 1, + }, + ) + self.contribution_type.accepted_evidence_url_types.set([github_type]) + evidence = submission.evidence_items.get() + evidence.url = 'https://github.com/a-different-owner/project' + evidence.url_type = github_type + evidence.save() + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(note), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + self.assertFalse(submission.more_info_responses.exists()) + + def test_legacy_request_is_snapshotted_without_fabricating_history(self): + submission, note = self._make_more_info_submission(structured=False) + self.assertIsNone(note) + + before = self.client.get(f'/api/v1/submissions/{submission.id}/') + self.assertEqual(before.status_code, status.HTTP_200_OK) + (active_request,) = before.data['more_info_requests'] + self.assertTrue(active_request['legacy']) + self.assertIsNone(active_request['response']) + self.assertEqual( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).count(), + 0, + ) + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(None), + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + stored = SubmissionMoreInfoResponse.objects.get( + submitted_contribution=submission, + ) + self.assertIsNone(stored.request_note) + self.assertEqual( + stored.request_message, + 'Please document the repository setup.', + ) + (paired_request,) = response.data['more_info_requests'] + self.assertTrue(paired_request['legacy']) + self.assertIsNotNone(paired_request['response']) + + def test_each_more_information_cycle_keeps_its_own_response(self): + submission, first_note = self._make_more_info_submission( + request_message='Add setup instructions.', + ) + first = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(first_note, 'Added setup instructions.'), + format='json', + ) + self.assertEqual(first.status_code, status.HTTP_200_OK, first.data) + + submission.refresh_from_db() + submission.state = 'more_info_needed' + submission.staff_reply = 'Now add a release link.' + submission.reviewed_by = self.steward_user + submission.reviewed_at = timezone.now() + submission.save(update_fields=[ + 'state', 'staff_reply', 'reviewed_by', 'reviewed_at', 'updated_at', + ]) + second_note = SubmissionNote.objects.create( + submitted_contribution=submission, + user=self.steward_user, + message='Reviewed: **more_info**\n\n> Now add a release link.', + data={ + 'action': 'more_info', + 'staff_reply': 'Now add a release link.', + }, + ) + + second = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(second_note, 'Added the release link.'), + format='json', + ) + + self.assertEqual(second.status_code, status.HTTP_200_OK, second.data) + self.assertEqual( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).count(), + 2, + ) + pairs = { + request['message']: request['response']['message'] + for request in second.data['more_info_requests'] + } + self.assertEqual(pairs, { + 'Add setup instructions.': 'Added setup instructions.', + 'Now add a release link.': 'Added the release link.', + }) + + def test_legacy_fallback_can_follow_an_answered_structured_cycle(self): + submission, first_note = self._make_more_info_submission( + request_message='Add setup instructions.', + ) + first = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(first_note, 'Added setup instructions.'), + format='json', + ) + self.assertEqual(first.status_code, status.HTTP_200_OK, first.data) + + submission.refresh_from_db() + submission.state = 'more_info_needed' + submission.staff_reply = 'Add deployment notes from the legacy review.' + submission.reviewed_by = self.steward_user + submission.reviewed_at = timezone.now() + submission.save(update_fields=[ + 'state', 'staff_reply', 'reviewed_by', 'reviewed_at', 'updated_at', + ]) + + second = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload(None, 'Added the deployment notes.'), + format='json', + ) + + self.assertEqual(second.status_code, status.HTTP_200_OK, second.data) + self.assertEqual( + SubmissionMoreInfoResponse.objects.filter( + submitted_contribution=submission, + ).count(), + 2, + ) + legacy_pair = next( + request + for request in second.data['more_info_requests'] + if request['legacy'] + ) + self.assertEqual( + legacy_pair['message'], + 'Add deployment notes from the legacy review.', + ) + self.assertEqual( + legacy_pair['response']['message'], + 'Added the deployment notes.', + ) + + def test_steward_search_matches_response_text(self): + submission, note = self._make_more_info_submission() + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + self._response_payload( + note, + 'Published the uncommon-response-token release notes.', + ), + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + + self.client.force_authenticate(user=self.steward_user) + search = self.client.get('/api/v1/steward-submissions/', { + 'state': 'pending', + 'search': 'uncommon-response-token', + }) + + self.assertEqual(search.status_code, status.HTTP_200_OK, search.data) + result_ids = {str(item['id']) for item in search.data['results']} + self.assertEqual(result_ids, {str(submission.id)}) diff --git a/backend/contributions/tests/test_state_transitions.py b/backend/contributions/tests/test_state_transitions.py index 46ddd07c..b0a86064 100644 --- a/backend/contributions/tests/test_state_transitions.py +++ b/backend/contributions/tests/test_state_transitions.py @@ -9,6 +9,7 @@ from contributions.models import ( Category, ContributionType, + Evidence, SubmissionNote, SubmissionStateTransition, SubmittedContribution, @@ -125,14 +126,26 @@ def test_bulk_reject_logs_transitions_and_decision_notes(self): def test_edit_after_more_info_logs_edited_transition(self): submission = self._make_submission( state='more_info_needed', + staff_reply='Please provide more context.', reviewed_by=self.steward_user, reviewed_at=timezone.now(), escalated_at=timezone.now(), ) + Evidence.objects.create( + submitted_contribution=submission, + description='Original evidence', + url='https://example.com/state-transition-evidence', + ) self.client.force_authenticate(user=self.owner) response = self.client.patch( f'/api/v1/submissions/{submission.id}/', - {'notes': 'Here is the extra information'}, + { + 'notes': 'Here is the extra information', + 'more_info_response': { + 'request_id': None, + 'message': 'Added the requested context.', + }, + }, format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/backend/contributions/tests/test_validator_category_gating.py b/backend/contributions/tests/test_validator_category_gating.py index 2f607efb..43fe0a4e 100644 --- a/backend/contributions/tests/test_validator_category_gating.py +++ b/backend/contributions/tests/test_validator_category_gating.py @@ -4,7 +4,13 @@ from rest_framework.test import APIClient from rest_framework import status -from contributions.models import Contribution, ContributionType, Category, SubmittedContribution +from contributions.models import ( + Category, + Contribution, + ContributionType, + Evidence, + SubmittedContribution, +) from creators.models import Creator from leaderboard.models import GlobalLeaderboardMultiplier from validators.models import Validator @@ -255,13 +261,13 @@ def test_plain_user_cannot_edit_submission_into_community_type(self): submission.refresh_from_db() self.assertEqual(submission.contribution_type, self.unrestricted_type) - def test_plain_user_can_edit_unchanged_legacy_community_submission(self): + def test_plain_user_can_edit_unchanged_legacy_community_pending_submission(self): submission = SubmittedContribution.objects.create( user=self.plain_user, contribution_type=self.community_type, contribution_date=timezone.now(), notes='Original legacy community submission', - state='more_info_needed', + state='pending', ) self.client.force_authenticate(user=self.plain_user) @@ -276,6 +282,37 @@ def test_plain_user_can_edit_unchanged_legacy_community_submission(self): self.assertEqual(submission.notes, 'Updated legacy community submission') self.assertEqual(submission.contribution_type, self.community_type) + def test_plain_user_cannot_resubmit_legacy_community_submission(self): + submission = SubmittedContribution.objects.create( + user=self.plain_user, + contribution_type=self.community_type, + contribution_date=timezone.now(), + notes='Original legacy community submission', + state='more_info_needed', + staff_reply='Please provide more context.', + ) + Evidence.objects.create( + submitted_contribution=submission, + description='Original evidence', + url='https://example.com/legacy-community-evidence', + ) + self.client.force_authenticate(user=self.plain_user) + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + { + 'more_info_response': { + 'request_id': None, + 'message': 'Added the requested context.', + }, + }, + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + submission.refresh_from_db() + self.assertEqual(submission.state, 'more_info_needed') + def test_plain_user_cannot_edit_submission_into_mission_only_type(self): submission = self._pending_submission() self.client.force_authenticate(user=self.plain_user) diff --git a/backend/contributions/views.py b/backend/contributions/views.py index 112b2553..49073503 100644 --- a/backend/contributions/views.py +++ b/backend/contributions/views.py @@ -27,7 +27,8 @@ from django.shortcuts import get_object_or_404 from .models import ( ContributionType, Contribution, Evidence, SubmittedContribution, - SubmissionNote, SubmissionStateTransition, ContributionHighlight, + SubmissionNote, SubmissionMoreInfoResponse, SubmissionStateTransition, + ContributionHighlight, Mission, StartupRequest, FeaturedContent, Alert, ContributionDiscordXPState, DiscordXPDistributionEvent, ProjectMilestoneReview, ReviewProposal, @@ -597,6 +598,15 @@ def get_queryset(self): ).select_related('user').order_by('-created_at', '-id'), to_attr='more_info_request_notes', ), + Prefetch( + 'more_info_responses', + queryset=SubmissionMoreInfoResponse.objects.select_related( + 'request_note', + 'requested_by', + 'responder', + ).order_by('-created_at', '-id'), + to_attr='more_info_response_rows', + ), ).order_by('-created_at') def get_serializer_context(self): @@ -965,92 +975,217 @@ def create(self, request, *args, **kwargs): def update(self, request, *args, **kwargs): """Update submission (only allowed if state is 'pending' or 'more_info_needed').""" partial = kwargs.pop('partial', False) - instance = self.get_object() - - # Check if update is allowed - if instance.state not in ['pending', 'more_info_needed']: - return Response( - {'error': 'Submission can only be edited when pending or when more information is requested.'}, - status=status.HTTP_403_FORBIDDEN + with transaction.atomic(): + # Serialize all edits/resubmissions for a submission. In particular, + # two clicks on Resubmit must never create two responses for one + # steward request. + instance = get_object_or_404( + SubmittedContribution.objects + .select_for_update(), + id=kwargs.get('pk'), + user=request.user, ) + self.check_object_permissions(request, instance) - # Appealed submissions are locked while awaiting re-review so the - # submitter can't edit-around the appeal. Once a steward explicitly - # asks for more info, edits are allowed again so the submitter can - # respond. - if instance.has_appeal and instance.state == 'pending': - return Response( - {'error': 'Appealed submissions cannot be edited while awaiting re-review.'}, - status=status.HTTP_403_FORBIDDEN - ) + # Check if update is allowed + if instance.state not in ['pending', 'more_info_needed']: + return Response( + {'error': 'Submission can only be edited when pending or when more information is requested.'}, + status=status.HTTP_403_FORBIDDEN + ) - if 'mission' in request.data: - requested_mission = request.data.get('mission') - current_mission = str(instance.mission_id) if instance.mission_id else None - if requested_mission in ('', None): - requested_mission = None - else: - requested_mission = str(requested_mission) - if requested_mission != current_mission: + # Appealed submissions are locked while awaiting re-review so the + # submitter can't edit-around the appeal. Once a steward explicitly + # asks for more info, edits are allowed again so the submitter can + # respond. + if instance.has_appeal and instance.state == 'pending': return Response( - {'error': 'Mission cannot be changed after submission.'}, - status=status.HTTP_400_BAD_REQUEST, + {'error': 'Appealed submissions cannot be edited while awaiting re-review.'}, + status=status.HTTP_403_FORBIDDEN ) - # Update the submission - serializer = self.get_serializer(instance, data=request.data, partial=partial) - serializer.is_valid(raise_exception=True) - was_more_info_needed = instance.state == 'more_info_needed' + if 'mission' in request.data: + requested_mission = request.data.get('mission') + current_mission = str(instance.mission_id) if instance.mission_id else None + if requested_mission in ('', None): + requested_mission = None + else: + requested_mission = str(requested_mission) + if requested_mission != current_mission: + return Response( + {'error': 'Mission cannot be changed after submission.'}, + status=status.HTTP_400_BAD_REQUEST, + ) - contribution_type = ( - serializer.validated_data.get('contribution_type') - or instance.contribution_type - ) - mission = serializer.validated_data.get('mission', instance.mission) - contribution_type = ( - ContributionType.objects - .select_related('category') - .prefetch_related('required_discord_roles') - .get(id=contribution_type.id) - ) - keeps_same_contribution_type = contribution_type.id == instance.contribution_type_id - contribution_type_error = self._validate_submission_contribution_type( - request.user, - contribution_type, - mission, - skip_capacity_check=keeps_same_contribution_type, - skip_weekly_capacity_check=keeps_same_contribution_type, - weekly_capacity_at=instance.created_at, - allow_existing_community_submission=keeps_same_contribution_type, - # A type made non-submittable after the fact must not lock users - # out of editing their existing pending/more-info submissions. - skip_submittable_check=keeps_same_contribution_type, - ) - if contribution_type_error is not None: - return contribution_type_error + serializer = self.get_serializer( + instance, + data=request.data, + partial=partial, + ) + serializer.is_valid(raise_exception=True) + was_more_info_needed = instance.state == 'more_info_needed' + response_snapshot = None - project_contribution_id = ( - request.data.get('project_contribution') - if 'project_contribution' in request.data - else (instance.project_contribution_id if instance.project_contribution_id else None) - ) - milestone_project_contribution, project_error = self._project_link_error( - request.user, - contribution_type, - project_contribution_id, - existing_project_contribution_id=instance.project_contribution_id, - ) - if project_error is not None: - return project_error + if was_more_info_needed: + response_payload = serializer.validated_data['more_info_response'] + latest_request = ( + SubmissionNote.objects + .select_for_update() + .select_related('user') + .filter( + submitted_contribution=instance, + is_proposal=False, + data__action='more_info', + ) + .order_by('-created_at', '-id') + .first() + ) - notes_error = self._milestone_notes_error( - contribution_type, - serializer.validated_data.get('notes', instance.notes), - ) - if notes_error is not None: - return notes_error + latest_request_answered = latest_request and ( + SubmissionMoreInfoResponse.objects.filter( + request_note=latest_request, + ).exists() + ) + if latest_request and not latest_request_answered: + if response_payload['request_id'] != latest_request.id: + return Response( + { + 'more_info_response': { + 'request_id': [ + 'This request is stale. Refresh the submission and respond to the latest request.' + ], + }, + }, + status=status.HTTP_409_CONFLICT, + ) + + note_data = latest_request.data or {} + request_message = ( + note_data.get('staff_reply') or latest_request.message + ) + response_snapshot = { + 'request_note': latest_request, + 'request_message': request_message, + 'requested_by': latest_request.user, + 'requested_at': latest_request.created_at, + } + else: + # Requests created before SubmissionNote tracking only + # survive in staff_reply. They use a null request_id and + # are snapshotted before that field is cleared. + if response_payload['request_id'] is not None: + if ( + latest_request_answered + and response_payload['request_id'] == latest_request.id + ): + detail = ( + 'This more-information request already has a response.' + ) + else: + detail = ( + 'This request is stale. Refresh the submission ' + 'and respond to the latest request.' + ) + return Response( + { + 'more_info_response': { + 'request_id': [detail], + }, + }, + status=status.HTTP_409_CONFLICT, + ) + if not (instance.staff_reply or '').strip(): + return Response( + { + 'more_info_response': { + 'request_id': [ + 'There is no active more-information request to answer.' + ], + }, + }, + status=status.HTTP_409_CONFLICT, + ) + response_snapshot = { + 'request_note': None, + 'request_message': instance.staff_reply, + 'requested_by': instance.reviewed_by, + 'requested_at': instance.reviewed_at, + } + + contribution_type = ( + serializer.validated_data.get('contribution_type') + or instance.contribution_type + ) + mission = serializer.validated_data.get('mission', instance.mission) + contribution_type = ( + ContributionType.objects + .select_related('category') + .prefetch_related('required_discord_roles') + .get(id=contribution_type.id) + ) + keeps_same_contribution_type = ( + contribution_type.id == instance.contribution_type_id + ) + contribution_type_error = self._validate_submission_contribution_type( + request.user, + contribution_type, + mission, + skip_capacity_check=keeps_same_contribution_type, + skip_weekly_capacity_check=keeps_same_contribution_type, + weekly_capacity_at=instance.created_at, + # Preserve legacy community pending-edit behavior, but a + # resubmission must satisfy current category authorization. + allow_existing_community_submission=( + keeps_same_contribution_type and not was_more_info_needed + ), + # A type made non-submittable after the fact must not lock users + # out of editing their existing pending/more-info submissions. + skip_submittable_check=keeps_same_contribution_type, + ) + if contribution_type_error is not None: + return contribution_type_error + + project_contribution_id = ( + request.data.get('project_contribution') + if 'project_contribution' in request.data + else ( + instance.project_contribution_id + if instance.project_contribution_id else None + ) + ) + milestone_project_contribution, project_error = self._project_link_error( + request.user, + contribution_type, + project_contribution_id, + existing_project_contribution_id=instance.project_contribution_id, + ) + if project_error is not None: + return project_error + + notes_error = self._milestone_notes_error( + contribution_type, + serializer.validated_data.get('notes', instance.notes), + ) + if notes_error is not None: + return notes_error + + # PATCH normally leaves evidence untouched. A more-info + # resubmission still has to validate the resulting stored snapshot + # against today's evidence, ownership, and duplicate rules. + if was_more_info_needed and 'evidence_items' not in request.data: + evidence_snapshot = list( + instance.evidence_items.values('id', 'description', 'url') + ) + serializer._validate_evidence_items( + evidence_snapshot, + require_at_least_one=( + not is_milestone_contribution_type(contribution_type) + ), + contribution_type=contribution_type, + user=request.user, + exclude_submission_id=instance.id, + ) - with transaction.atomic(): locked_type = ( ContributionType.objects .select_for_update() @@ -1105,16 +1240,23 @@ def update(self, request, *args, **kwargs): serializer.validated_data['project_contribution'] = None serializer.validated_data['milestone_version'] = None - # Update state back to pending and track edit time instance.state = 'pending' instance.last_edited_at = timezone.now() - instance.staff_reply = '' # Clear previous staff reply + instance.staff_reply = '' instance.gate_reviewed = False instance.reviewed_by = None instance.reviewed_at = None self.perform_update(serializer) + if was_more_info_needed: + SubmissionMoreInfoResponse.objects.create( + submitted_contribution=instance, + responder=request.user, + message=response_payload['message'], + **response_snapshot, + ) + SubmissionStateTransition.record( instance, SubmissionStateTransition.EVENT_EDITED, @@ -1130,7 +1272,12 @@ def update(self, request, *args, **kwargs): actor=request.user, ) - return Response(serializer.data) + # Re-fetch so the response just created is included in the nested + # request history returned to the client. + response_instance = self.get_queryset().get(id=instance.id) + response_data = self.get_serializer(response_instance).data + + return Response(response_data) def destroy(self, request, *args, **kwargs): """ @@ -1445,13 +1592,18 @@ def _content_query(self, term): converted_evidence = Evidence.objects.filter( contribution_id=OuterRef('converted_contribution_id') ).filter(self._evidence_content_query(term)) + submitter_responses = SubmissionMoreInfoResponse.objects.filter( + submitted_contribution_id=OuterRef('pk'), + message__icontains=term, + ) return ( Q(title__icontains=term) | Q(notes__icontains=term) | Q(converted_contribution__title__icontains=term) | Q(converted_contribution__notes__icontains=term) | Exists(submitted_evidence) | - Exists(converted_evidence) + Exists(converted_evidence) | + Exists(submitter_responses) ) def filter_search(self, queryset, name, value): @@ -2375,6 +2527,15 @@ def get_queryset(self): ).select_related('user').order_by('-created_at', '-id'), to_attr='more_info_request_notes', ), + Prefetch( + 'more_info_responses', + queryset=SubmissionMoreInfoResponse.objects.select_related( + 'request_note', + 'requested_by', + 'responder', + ).order_by('-created_at', '-id'), + to_attr='more_info_response_rows', + ), Prefetch( 'review_proposals', queryset=ReviewProposal.objects.filter( diff --git a/backend/notifications/services.py b/backend/notifications/services.py index 4a30c6e3..6d238de8 100644 --- a/backend/notifications/services.py +++ b/backend/notifications/services.py @@ -454,8 +454,8 @@ def notify_submission_reopened_for_steward(submission, *, kind, actor=None): dedupe_marker = 'appeal' elif kind == 'more_info_resubmitted': event_slug = 'submission.more_info_resubmitted' - title = 'More information resubmitted' - body = f"{name} was updated by the submitter after your more-information request." + title = 'More-information response received' + body = f"The submitter responded to your more-information request for {name}." edit_at = submission.last_edited_at or submission.updated_at dedupe_marker = edit_at.isoformat() if edit_at else '' else: diff --git a/backend/notifications/tests.py b/backend/notifications/tests.py index c9f1018b..54b5ea26 100644 --- a/backend/notifications/tests.py +++ b/backend/notifications/tests.py @@ -6,7 +6,7 @@ from django.utils import timezone from rest_framework.test import APIClient -from contributions.models import Category, ContributionType, SubmittedContribution +from contributions.models import Category, ContributionType, Evidence, SubmittedContribution from partners.models import Partner from validators.models import Validator @@ -490,11 +490,22 @@ def test_more_info_resubmission_notifies_assigned_steward(self): assigned_to=self.steward_user, gate_reviewed=True, ) + Evidence.objects.create( + submitted_contribution=submission, + description='Original evidence', + url='https://example.com/notification-resubmit-evidence', + ) self.client.force_authenticate(user=self.submitter) response = self.client.patch( f'/api/v1/submissions/{submission.id}/', - {'notes': 'Added the requested evidence.'}, + { + 'notes': 'Added the requested evidence.', + 'more_info_response': { + 'request_id': None, + 'message': 'Added the requested evidence.', + }, + }, format='json', ) diff --git a/backend/submissions_review.md b/backend/submissions_review.md index e89cecbc..67832973 100644 --- a/backend/submissions_review.md +++ b/backend/submissions_review.md @@ -114,6 +114,16 @@ same structured notes rendered in submission cards, from `is_more_info_resubmitted`, backed only by an append-only transition from `more_info_needed` to `pending`. +Each `more_info_requests[]` item also contains a nullable `response` object +(`id`, `message`, `user`, `user_name`, `created_at`). A submitter answers the +latest unanswered request by including +`more_info_response: {request_id, message}` in the normal owner-scoped +`PUT/PATCH /api/v1/submissions/{id}/` update. The response is required for the +`more_info_needed -> pending` transition, limited to 1,000 characters, paired +to that request cycle, and saved atomically with current submission validation. +For a legacy request sourced only from `staff_reply`, `request_id` is `null`; +no historical response is inferred. + Appeals and more-info resubmissions remain available to both AI stages. The deterministic gate records an appeal as gate-reviewed but never auto-rejects it, so an appeal always reaches a human reviewer. diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 58ea4d9a..45a450c8 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -392,7 +392,7 @@ const routes = { '/validators/contribution/:id': ContributionPreview, // Validator-scoped contribution detail '/contribution-type/:id': ContributionTypeDetail, '/badge/:id': BadgeDetail, - '/submit-contribution': SubmitContribution, + '/submit-contribution': SubmitContribution, // ?resubmit= owner-loads and prefills a normal create; takes precedence over ?mission and ?type '/my-submissions': MySubmissions, '/contributions/:id': EditSubmission, '/metrics': Metrics, @@ -536,8 +536,8 @@ In production the SPA (Amplify) and the API are on different hosts and `CSRF_COO #### Steward Review Components (`src/components/`) -- **`SubmissionCard.svelte`** - Submitter-only card used by My Submissions. It shows submission details, evidence, staff responses, awarded contributions, editing, and appeals without exposing steward-only state. -- **`StewardSubmissionCard.svelte`** - Dedicated steward workspace for review outcomes, proposals, rubric scoring, internal notes, accepted-contribution edits, and permission-aware submission behavior. It marks `escalated_at` proposals and previews `escalation_threshold_points` conversion for reviewer-tier accepts. +- **`SubmissionCard.svelte`** - Submitter-only card used by My Submissions. More-info cards keep Edit and add an inline required “What did you change?” direct-resubmit action; rejected cards keep Appeal and add a corrected-submission link to `/submit-contribution?resubmit=`. Request responses render in their own nested box, separate from original notes. +- **`StewardSubmissionCard.svelte`** - Dedicated steward workspace for review outcomes, proposals, rubric scoring, internal notes, accepted-contribution edits, and permission-aware submission behavior. It marks `escalated_at` proposals, previews `escalation_threshold_points` conversion for reviewer-tier accepts, and renders each submitter response beneath its paired more-info request. - **Contribution type review fields** - Contribution type payloads expose `requires_ai_review` and nullable `escalation_threshold_points`; the latter drives the card's advisory preview while the server remains authoritative. - **Steward submission search** - `is:escalated` / `not:escalated` map to the `is_escalated` API filter. Tier 2+ stewards get an Escalated queue chip; tier 3 stewards also get an Apex queue chip for `status:accepted is:interesting`. - **`AIReviewSummary.svelte`** - Compact AI-only proposal summary that exposes the proposed action, confidence, and expandable synthesis. @@ -662,7 +662,8 @@ Investor-oriented home page (`routes/Overview.svelte`), top to bottom: hero → - Uses VITE_RECAPTCHA_SITE_KEY from environment (falls back to test key) - Honors optional per-user Monday-Sunday UTC contribution-type limits from `user_weekly_*` API fields and shows the user's remaining weekly capacity. - New Milestones links can select only highlighted Projects contributions. `EditSubmission.svelte` passes the current submission ID so a pre-policy pending milestone keeps its existing unhighlighted link. -- `EditSubmission.svelte` - Edit submitted contributions (supports URL and description evidence only - no file uploads) + - Rejected resubmission is prefill context only: `?resubmit=` clones date, title, notes, evidence values, type, mission, and linked project without evidence database IDs, then submits through the ordinary POST + reCAPTCHA path. The old rejected row remains untouched. Retired/full types and inactive/full missions preserve copied content but require a valid explicit selection; no silent fallback is allowed. A successful clone returns to `?submission=` for highlighting. +- `EditSubmission.svelte` - Edit submitted contributions (supports URL and description evidence only - no file uploads). A more-info edit shows the steward request plus a distinct required response panel, sends `more_info_response: {request_id, message}`, and labels the action “Save and resubmit.” - `ProfileEdit.svelte` - User profile editing (name and profile fields; node version shown read-only, Grafana-sourced) - `Profile.svelte` - Public participant profile view diff --git a/frontend/src/components/StewardSubmissionCard.svelte b/frontend/src/components/StewardSubmissionCard.svelte index bda1f4a3..65e72f0f 100644 --- a/frontend/src/components/StewardSubmissionCard.svelte +++ b/frontend/src/components/StewardSubmissionCard.svelte @@ -1596,6 +1596,22 @@ {request.created_at ? ' on ' + formatDate(request.created_at) : ''}

{/if} + {#if request.response} +
+
+

Submitter response

+ {#if request.response.created_at} +

{formatDate(request.response.created_at)}

+ {/if} +
+
+ {@html parseUserMarkdown(request.response.message)} +
+ {#if request.response.user_name} +

Responded by {request.response.user_name}

+ {/if} +
+ {/if} {/each} diff --git a/frontend/src/components/SubmissionCard.svelte b/frontend/src/components/SubmissionCard.svelte index 78394ceb..8fa7f114 100644 --- a/frontend/src/components/SubmissionCard.svelte +++ b/frontend/src/components/SubmissionCard.svelte @@ -9,9 +9,10 @@ /** @type {{ * submission: Record, - * onAppeal?: ((submissionId: string | number, reason: string) => Promise) | null + * onAppeal?: ((submissionId: string | number, reason: string) => Promise) | null, + * onResubmit?: ((submissionId: string | number, response: { request_id: number | null, message: string }) => Promise) | null * }} */ - let { submission, onAppeal = null } = $props(); + let { submission, onAppeal = null, onResubmit = null } = $props(); /** @type {Record} */ const STATE_STYLES = { @@ -50,6 +51,10 @@ let appealReason = $state(''); let submittingAppeal = $state(false); let copyingSubmissionId = $state(false); + let showingResubmitResponse = $state(false); + let moreInfoResponse = $state(''); + let resubmitting = $state(false); + let resubmitError = $state(''); let stateStyle = $derived(STATE_STYLES[submission.state] || DEFAULT_STATE_STYLE); let contributionTypeName = $derived( @@ -75,6 +80,11 @@ (/** @type {Record} */ request) => request?.message ) ); + let activeMoreInfoRequest = $derived( + moreInfoRequests.find( + (/** @type {Record} */ request) => !request.response + ) || null + ); let showStaffResponse = $derived(Boolean( submission.staff_reply && submission.state !== 'rejected' && @@ -139,10 +149,53 @@ } } + /** @param {any} error */ + function moreInfoErrorMessage(error) { + const responseData = error?.response?.data || {}; + const responseError = responseData.more_info_response; + if (typeof responseError === 'string') return responseError; + if (Array.isArray(responseError)) return responseError[0]; + if (responseError && typeof responseError === 'object') { + const firstValue = Object.values(responseError)[0]; + if (Array.isArray(firstValue)) return firstValue[0]; + if (typeof firstValue === 'string') return firstValue; + } + const evidenceError = responseData.evidence_items; + if (typeof evidenceError === 'string') return evidenceError; + if (Array.isArray(evidenceError) && evidenceError.length > 0) { + return evidenceError[0]?.message || String(evidenceError[0]); + } + return responseData.error || responseData.detail || 'We could not resubmit this contribution. Open Edit to review the current requirements.'; + } + + async function handleMoreInfoResubmit() { + const message = moreInfoResponse.trim(); + if (!onResubmit || !message || resubmitting) return; + + resubmitting = true; + resubmitError = ''; + try { + await onResubmit(submission.id, { + request_id: activeMoreInfoRequest?.id ?? null, + message + }); + moreInfoResponse = ''; + showingResubmitResponse = false; + } catch (error) { + resubmitError = moreInfoErrorMessage(error); + } finally { + resubmitting = false; + } + } + function editSubmission() { const missionQuery = submission.mission?.id ? `?mission=${submission.mission.id}` : ''; push(`/contributions/${submission.id}${missionQuery}`); } + + function resubmitRejectedSubmission() { + push(`/submit-contribution?resubmit=${encodeURIComponent(submission.id)}`); + }
@@ -268,6 +321,19 @@ {request.user_name ? `Requested by ${request.user_name}` : 'Requested'} {#if request.created_at}on {formatDate(request.created_at)}{/if}

+ {#if request.response} +
+
+

Your response

+ {#if request.response.created_at} +

{formatDate(request.response.created_at)}

+ {/if} +
+
+ {@html parseUserMarkdown(request.response.message)} +
+
+ {/if}
{/each} @@ -313,6 +379,20 @@ {/if} +
+

Submit a corrected contribution

+

+ Resubmit creates a new, editable contribution using these details. This rejected submission stays unchanged{submission.has_appeal ? '.' : ' and can still be appealed.'} +

+ +
+ {#if submission.has_appeal}

@@ -355,7 +435,7 @@

Your appeal is under review

A steward will re-review your submission.

- {:else if submission.state === 'pending' || submission.state === 'more_info_needed'} + {:else if submission.state === 'pending'}
+ {:else if submission.state === 'more_info_needed'} +
+
+ + {#if onResubmit} + + {/if} +
+ + {#if showingResubmitResponse} +
+ +

+ Briefly tell the steward how you addressed the request. If the submission itself also needs edits, use Edit instead. +

+ +
+
+ {#if resubmitError} + + {/if} +
+

{moreInfoResponse.length}/1000

+
+
+ +
+
+ {/if} +
{/if} diff --git a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte index c006e98b..9c2c9479 100644 --- a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte +++ b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte @@ -22,6 +22,7 @@ missionId = null, initialTypeId = null, submission = null, + resubmitSource = null, } = $props(); let submitting = $state(false); @@ -33,6 +34,8 @@ let isBuilder = $derived(!!$userStore.user?.builder); let isCreator = $derived(!!$userStore.user?.creator); let editMode = $derived(Boolean(submission?.id)); + let resubmitMode = $derived(!editMode && Boolean(resubmitSource?.id)); + let cloneContextWarning = $state(""); // reCAPTCHA state let recaptchaToken = $state(""); @@ -91,6 +94,11 @@ let originalContributionTypeId = $derived( submission?.contribution_type ?? null, ); + let resubmitSourceProjectId = $derived( + resubmitSource?.project_contribution?.id ?? + resubmitSource?.project_contribution ?? + "", + ); let keepsOriginalContributionType = $derived( editMode && selectedType && @@ -98,6 +106,7 @@ ); let hasLegacyCommunityEditAccess = $derived( editMode && + submission?.state !== "more_info_needed" && submission?.contribution_type_details?.category === "community", ); let canAccessCurrentCategory = $derived( @@ -113,7 +122,8 @@ hasCurrentCategoryRole || ( keepsOriginalContributionType && - selectedCategory === "community" + selectedCategory === "community" && + hasLegacyCommunityEditAccess ), ); let selectedMission = $state(null); @@ -124,11 +134,18 @@ ); let selectionLocked = $derived(missionLocked || appealLocked); let latestMoreInfoRequest = $derived( - submission?.more_info_requests?.[0] || null, + submission?.more_info_requests?.find( + (/** @type {Record} */ request) => !request?.response, + ) || + submission?.more_info_requests?.[0] || + null, ); let reviewFeedback = $derived( latestMoreInfoRequest?.message || submission?.staff_reply || "", ); + let requiresMoreInfoResponse = $derived( + editMode && submission?.state === "more_info_needed", + ); let acceptedProjects = $state([]); let loadingProjects = $state(false); let projectsError = $state(false); @@ -145,6 +162,7 @@ title: "", notes: "", }); + let moreInfoResponse = $state(""); // Evidence Slots let evidenceSlots = $state([]); @@ -198,6 +216,15 @@ }; } + /** @param {Array> | null | undefined} items */ + function evidenceItemsWithoutIds(items) { + return (items || []).map((/** @type {Record} */ item) => ({ + description: item?.description || "", + url: item?.url || "", + url_type: item?.url_type || null, + })); + } + function evidenceMatchesRequiredType(slot, contributionType) { const requiredTypes = contributionType?.required_evidence_url_types || []; if (!slot?.selectedType || requiredTypes.length === 0) return false; @@ -243,26 +270,27 @@ let disposed = false; async function initializeForm() { - if (editMode) { + if (editMode || resubmitMode) { + const source = editMode ? submission : resubmitSource; selectedCategory = - submission.contribution_type_details?.category || "builder"; + source.contribution_type_details?.category || "builder"; selectedMission = - submission.mission?.id ?? submission.mission ?? null; + source.mission?.id ?? source.mission ?? null; selectedProject = - submission.project_contribution?.id ?? - submission.project_contribution ?? + source.project_contribution?.id ?? + source.project_contribution ?? ""; searchQuery = - submission.contribution_type_name || - submission.contribution_type_details?.name || + source.contribution_type_name || + source.contribution_type_details?.name || ""; formData = { - contribution_type: submission.contribution_type || "", + contribution_type: source.contribution_type || "", contribution_date: - submission.contribution_date?.split("T")[0] || + source.contribution_date?.split("T")[0] || new Date().toISOString().split("T")[0], - title: submission.title || "", - notes: submission.notes || "", + title: source.title || "", + notes: source.notes || "", }; } @@ -343,7 +371,127 @@ } } } - // Handle pre-selection from URL params + // Handle rejected-submission cloning before ordinary URL + // preselection. The cloned text/evidence always stays populated, + // while an unavailable type or mission is deliberately left for + // the user to replace. + } else if (resubmitMode) { + const clonedEvidence = evidenceItemsWithoutIds( + resubmitSource.evidence_items, + ); + let sourceType = types.find( + (type) => + String(type.id) === String(resubmitSource.contribution_type), + ); + if (!sourceType) { + try { + const response = await contributionsAPI.getContributionType( + resubmitSource.contribution_type, + ); + sourceType = response.data || null; + if (sourceType) { + types = [...types, sourceType]; + allEvidenceUrlTypes = collectEvidenceUrlTypes(types); + } + } catch (err) { + sourceType = null; + } + } + + partitionEvidenceForType(sourceType, clonedEvidence); + if (!sourceType) { + selectedType = null; + selectedMission = null; + selectedMissionData = null; + formData.contribution_type = ""; + searchQuery = ""; + showTypeDropdown = true; + cloneContextWarning = `The original contribution type, ${resubmitSource.contribution_type_name || "Unknown type"}, is no longer available. Choose a current type to continue.`; + } else { + selectedCategory = sourceType.category || selectedCategory; + const sourceMissionId = + resubmitSource.mission?.id ?? resubmitSource.mission ?? null; + + if (sourceMissionId) { + let sourceMission = missions.find( + (mission) => String(mission.id) === String(sourceMissionId), + ); + if (!sourceMission) { + try { + const response = await contributionsAPI.getMission( + sourceMissionId, + ); + sourceMission = response.data || null; + } catch (err) { + sourceMission = null; + } + } + + if ( + sourceMission && + String(sourceMission.contribution_type) === + String(sourceType.id) && + isMissionSubmittable(sourceMission) && + !isTypeFull(sourceType) + ) { + selectedType = sourceType; + selectedMission = sourceMission.id; + selectedMissionData = sourceMission; + formData.contribution_type = sourceType.id; + searchQuery = sourceMission.name; + } else { + selectedType = null; + selectedMission = null; + selectedMissionData = null; + formData.contribution_type = ""; + searchQuery = ""; + showTypeDropdown = true; + const missionName = + sourceMission?.name || + resubmitSource.mission?.name || + "The original mission"; + cloneContextWarning = isTypeFull(sourceType) + ? `${sourceType.name} is currently full. Your copied details are intact; choose an available contribution type or mission.` + : `${missionName} is no longer accepting submissions. Your copied details are intact; choose a current contribution type or mission.`; + } + } else if (sourceType.is_submittable && !isTypeFull(sourceType)) { + selectedType = sourceType; + selectedMission = null; + selectedMissionData = null; + formData.contribution_type = sourceType.id; + searchQuery = sourceType.name; + } else { + selectedType = null; + selectedMission = null; + selectedMissionData = null; + formData.contribution_type = ""; + searchQuery = ""; + showTypeDropdown = true; + cloneContextWarning = isTypeFull(sourceType) + ? `${sourceType.name} is currently full. Your copied details are intact; choose an available contribution type or mission.` + : `${sourceType.name} no longer accepts direct submissions. Your copied details are intact; choose a current contribution type or mission.`; + } + + if (selectedType && isMilestoneType(selectedType)) { + const sourceProjectId = + resubmitSource.project_contribution?.id ?? + resubmitSource.project_contribution ?? + ""; + const projectStillAvailable = acceptedProjects.some( + (project) => String(project.id) === String(sourceProjectId), + ); + selectedProject = projectStillAvailable + ? String(sourceProjectId) + : ""; + if (sourceProjectId && !projectStillAvailable) { + const projectWarning = + "The originally linked project is no longer available for a milestone submission. Select one of your current highlighted projects."; + cloneContextWarning = cloneContextWarning + ? `${cloneContextWarning} ${projectWarning}` + : projectWarning; + } + } + } } else if (missionId) { // Load the specific mission and pre-select it try { @@ -444,7 +592,9 @@ } catch (err) { error = editMode ? "Failed to load this submission's form." - : "Failed to load contribution categories."; + : resubmitMode + ? "Failed to prepare this rejected submission. Please try again." + : "Failed to load contribution categories."; console.error(err); } finally { if (!disposed) loadingTypes = false; @@ -649,6 +799,18 @@ return ["projects", "projects-and-milestones", "projects-milestones", "project-milestone"].includes(type?.slug); } + /** @param {Record | null} type */ + function clonedProjectForType(type) { + if (!resubmitMode || !isMilestoneType(type) || !resubmitSourceProjectId) { + return ""; + } + return acceptedProjects.some( + (project) => String(project.id) === String(resubmitSourceProjectId), + ) + ? String(resubmitSourceProjectId) + : ""; + } + async function loadAcceptedProjects() { if (!$authState.isAuthenticated) return; loadingProjects = true; @@ -852,11 +1014,12 @@ selectedType = t; selectedMission = null; selectedMissionData = null; - selectedProject = ""; + selectedProject = clonedProjectForType(t); showProjectDropdown = false; formData.contribution_type = t.id; showTypeDropdown = false; searchQuery = t.name; + if (resubmitMode) cloneContextWarning = ""; if (error === "Please select a contribution type") error = ""; } @@ -885,11 +1048,12 @@ selectedType = item.parentType; selectedMission = item.data.id; selectedMissionData = item.data; - selectedProject = ""; + selectedProject = clonedProjectForType(item.parentType); showProjectDropdown = false; formData.contribution_type = item.parentType.id; showTypeDropdown = false; searchQuery = item.data.name; + if (resubmitMode) cloneContextWarning = ""; if (error === "Please select a contribution type") error = ""; } } @@ -1296,6 +1460,11 @@ return; } + if (requiresMoreInfoResponse && !moreInfoResponse.trim()) { + error = "Tell the steward what changed before resubmitting."; + return; + } + if (!formData.contribution_type) { trackEvent(editMode ? "contribution_edit_error" : "contribution_submit_error", getAnalyticsContext({ surface: editMode ? "edit_form" : "form", @@ -1434,6 +1603,12 @@ title: formData.title, notes: formData.notes, }; + if (requiresMoreInfoResponse) { + submissionData.more_info_response = { + request_id: latestMoreInfoRequest?.id ?? null, + message: moreInfoResponse.trim(), + }; + } if (!editMode) submissionData.recaptcha = currentRecaptchaToken; if (isMilestoneType(selectedType)) { @@ -1458,11 +1633,9 @@ // Send evidence inline with the submission (atomic creation) submissionData.evidence_items = allEvidence; - if (editMode) { - await api.put(`/submissions/${submission.id}/`, submissionData); - } else { - await api.post("/submissions/", submissionData); - } + const savedResponse = editMode + ? await api.put(`/submissions/${submission.id}/`, submissionData) + : await api.post("/submissions/", submissionData); if (editMode) { trackEvent("contribution_edit_success", getAnalyticsContext({ @@ -1492,10 +1665,19 @@ sessionStorage.setItem( "submissionUpdateSuccess", editMode - ? "Your submission has been saved successfully." - : "Your contribution has been submitted successfully and is pending review.", + ? requiresMoreInfoResponse + ? "Your response was sent and the submission is back in review." + : "Your submission has been saved successfully." + : resubmitMode + ? "Your corrected contribution has been submitted and is pending review." + : "Your contribution has been submitted successfully and is pending review.", + ); + const createdSubmissionId = resubmitMode ? savedResponse?.data?.id : null; + push( + createdSubmissionId + ? `/my-submissions?submission=${encodeURIComponent(createdSubmissionId)}` + : "/my-submissions", ); - push("/my-submissions"); } catch (err) { trackEvent(editMode ? "contribution_edit_error" : "contribution_submit_error", getAnalyticsContext({ surface: editMode ? "edit_form" : "form", @@ -1518,6 +1700,16 @@ ? evidenceErrors : JSON.stringify(evidenceErrors); } + } else if (err.response?.data?.more_info_response) { + const responseError = err.response.data.more_info_response; + if (typeof responseError === "string") { + error = responseError; + } else if (Array.isArray(responseError)) { + error = responseError[0]; + } else { + const firstValue = Object.values(responseError)[0]; + error = Array.isArray(firstValue) ? firstValue[0] : firstValue; + } } else { error = err.response?.data?.error || @@ -1589,7 +1781,7 @@
{#if editMode}
@@ -1671,6 +1863,74 @@
{/if} + + {#if requiresMoreInfoResponse} +
+ +

+ This response is sent separately from your original submission notes, so the steward can see exactly how you addressed the request. +

+ +

+ {moreInfoResponse.length}/1000 +

+
+ {/if} + {:else if resubmitMode} +
+ +
+

+ Corrected submission +

+

+ Resubmit contribution +

+

+ Review and update this copied content before creating a new submission. The rejected original and its appeal history remain unchanged. +

+
+
+ + {#if resubmitSource.staff_reply} +
+

Original rejection reason

+ +
+ {/if} + + {#if cloneContextWarning} +
+ +

{cloneContextWarning}

+
+ {/if} {:else}

{#if submitting} - {editMode ? "Saving..." : "Submitting..."} + {editMode + ? requiresMoreInfoResponse + ? "Resubmitting..." + : "Saving..." + : "Submitting..."} {:else} - {editMode ? "Save changes" : "Submit Contribution"} + {editMode + ? requiresMoreInfoResponse + ? "Save and resubmit" + : "Save changes" + : resubmitMode + ? "Resubmit Contribution" + : "Submit Contribution"} {/if} {#if !submitting} @@ -2660,7 +2930,7 @@ + {/if} + + + + {:else} - + {/if} diff --git a/frontend/src/tests/EditSubmission.test.js b/frontend/src/tests/EditSubmission.test.js index 12689ef6..99042d40 100644 --- a/frontend/src/tests/EditSubmission.test.js +++ b/frontend/src/tests/EditSubmission.test.js @@ -60,7 +60,7 @@ vi.mock('../lib/userStore.js', async () => { userStore: readable({ user: { address: '0xcommunity', - builder: false, + builder: true, validator: false, creator: false, twitter_connection: null, @@ -331,6 +331,53 @@ describe('EditSubmission', () => { expect(push).toHaveBeenCalledWith('/my-submissions'); }); + it('sends a required response separately when saving a more-info edit', async () => { + renderEditor(makeSubmission({ + state: 'more_info_needed', + contribution_type: builderType.id, + contribution_type_name: builderType.name, + contribution_type_details: { + id: builderType.id, + name: builderType.name, + slug: builderType.slug, + category: builderType.category + }, + staff_reply: 'Add release documentation.', + more_info_requests: [{ + id: 37, + message: 'Add release documentation.', + user_name: 'Builder Steward', + created_at: '2026-07-19T12:00:00Z', + response: null + }] + })); + + expect(await screen.findByText('Changes requested')).toBeTruthy(); + expect(screen.getByText('Add release documentation.')).toBeTruthy(); + const typeInput = await screen.findByDisplayValue('Builder Project'); + await waitFor(() => expect(typeInput.disabled).toBe(false)); + const responseInput = screen.getByRole('textbox', { name: 'What did you change?' }); + expect(screen.getByRole('button', { name: 'Save and resubmit' })).toBeTruthy(); + + await fireEvent.input(responseInput, { + target: { value: ' Added release and setup documentation. ' } + }); + await fireEvent.click(screen.getByRole('button', { name: 'Save and resubmit' })); + + await waitFor(() => { + expect(mocks.api.put).toHaveBeenCalledTimes(1); + }); + const [, payload] = mocks.api.put.mock.calls[0]; + expect(payload.more_info_response).toEqual({ + request_id: 37, + message: 'Added release and setup documentation.' + }); + expect(payload.notes).toBe('Hosted the weekly call and published the recording.'); + expect(payload.evidence_items[0]).toMatchObject({ id: 81 }); + expect(payload).not.toHaveProperty('recaptcha'); + expect(sessionStorage.getItem('submissionUpdateSuccess')).toContain('back in review'); + }); + it('removes an editable submission from the branded confirmation dialog', async () => { renderEditor(); await screen.findByRole('button', { name: 'Save changes' }); diff --git a/frontend/src/tests/StewardSubmissionCard.test.js b/frontend/src/tests/StewardSubmissionCard.test.js index b7e05d45..6bf2e8ec 100644 --- a/frontend/src/tests/StewardSubmissionCard.test.js +++ b/frontend/src/tests/StewardSubmissionCard.test.js @@ -196,6 +196,30 @@ describe('StewardSubmissionCard', () => { expect(screen.getByText('The rejection overlooked the attached evidence.')).toBeTruthy(); }); + it('pairs a more-information request with its distinct submitter response', () => { + renderCard({ + submission: makeSubmission({ + more_info_requests: [{ + id: 31, + message: 'Add installation instructions.', + user_name: 'Review Steward', + created_at: '2026-06-02T12:00:00Z', + response: { + id: 44, + message: 'Added installation and environment setup instructions.', + user_name: 'Project Builder', + created_at: '2026-06-03T12:00:00Z' + } + }] + }) + }); + + expect(screen.getByText('Add installation instructions.')).toBeTruthy(); + expect(screen.getByText('Submitter response')).toBeTruthy(); + expect(screen.getByText('Added installation and environment setup instructions.')).toBeTruthy(); + expect(screen.getByText('Responded by Project Builder')).toBeTruthy(); + }); + it('shows the compact AI proposal context without a competing human proposal', () => { const aiAnalysis = makeAIAnalysis(); renderCard({ diff --git a/frontend/src/tests/SubmissionCard.test.js b/frontend/src/tests/SubmissionCard.test.js index 093db477..dcb2a05a 100644 --- a/frontend/src/tests/SubmissionCard.test.js +++ b/frontend/src/tests/SubmissionCard.test.js @@ -198,6 +198,105 @@ describe('SubmissionCard', () => { expect(screen.queryByText('Staff Response')).toBeNull(); }); + it('renders each submitter response separately from the steward request', () => { + render(SubmissionCard, { + props: { + submission: makeSubmission({ + state: 'pending', + more_info_requests: [{ + id: 2, + message: 'Please add repository setup instructions.', + user_name: 'Test Steward', + created_at: '2026-06-02T12:00:00Z', + response: { + id: 4, + message: 'Added a setup section to the README.', + user_name: 'Project Builder', + created_at: '2026-06-03T12:00:00Z' + } + }] + }) + } + }); + + expect(screen.getByText('Please add repository setup instructions.')).toBeTruthy(); + expect(screen.getByText('Your response')).toBeTruthy(); + expect(screen.getByText('Added a setup section to the README.')).toBeTruthy(); + }); + + it('requires an inline response before directly resubmitting unchanged details', async () => { + const onResubmit = vi.fn().mockResolvedValue(); + render(SubmissionCard, { + props: { + submission: makeSubmission({ + state: 'more_info_needed', + state_display: 'More Information Needed', + more_info_requests: [{ + id: 23, + message: 'Document the latest repository changes.', + response: null + }] + }), + onResubmit + } + }); + + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit' })); + const responseInput = screen.getByRole('textbox', { name: 'What did you change?' }); + const sendButton = screen.getByRole('button', { name: 'Send response and resubmit' }); + expect(responseInput.required).toBe(true); + expect(responseInput.maxLength).toBe(1000); + expect(sendButton.disabled).toBe(true); + + await fireEvent.input(responseInput, { + target: { value: ' Updated the repository and release notes. ' } + }); + expect(sendButton.disabled).toBe(false); + await fireEvent.click(sendButton); + + await waitFor(() => { + expect(onResubmit).toHaveBeenCalledWith(42, { + request_id: 23, + message: 'Updated the repository and release notes.' + }); + }); + }); + + it('keeps the inline response open and surfaces resubmission errors', async () => { + const onResubmit = vi.fn().mockRejectedValue({ + response: { + data: { + evidence_items: 'At least one current evidence URL is required.' + } + } + }); + render(SubmissionCard, { + props: { + submission: makeSubmission({ + state: 'more_info_needed', + state_display: 'More Information Needed', + more_info_requests: [{ + id: 23, + message: 'Add current evidence.', + response: null + }] + }), + onResubmit + } + }); + + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit' })); + await fireEvent.input(screen.getByRole('textbox', { name: 'What did you change?' }), { + target: { value: 'Updated the repository evidence.' } + }); + await fireEvent.click(screen.getByRole('button', { name: 'Send response and resubmit' })); + + expect((await screen.findByRole('alert')).textContent).toContain( + 'At least one current evidence URL is required.' + ); + expect(screen.getByRole('textbox', { name: 'What did you change?' })).toBeTruthy(); + }); + it('shows the final staff response and awarded contribution after acceptance', () => { render(SubmissionCard, { props: { @@ -254,6 +353,25 @@ describe('SubmissionCard', () => { expect(screen.getByLabelText('Appeal reason').value).toBe(''); }); + it('keeps Appeal available while routing rejected Resubmit to a new form', async () => { + render(SubmissionCard, { + props: { + submission: makeSubmission({ + id: 'rejected-42', + state: 'rejected', + state_display: 'Rejected', + staff_reply: 'Correct the evidence and try again.' + }), + onAppeal: vi.fn() + } + }); + + expect(screen.getByRole('button', { name: 'Submit Appeal' })).toBeTruthy(); + expect(screen.getByText(/creates a new, editable contribution/)).toBeTruthy(); + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit' })); + expect(push).toHaveBeenCalledWith('/submit-contribution?resubmit=rejected-42'); + }); + it('does not offer a second appeal and shows an appeal under review', () => { const { unmount } = render(SubmissionCard, { props: { diff --git a/frontend/src/tests/SubmitContributionResubmit.test.js b/frontend/src/tests/SubmitContributionResubmit.test.js new file mode 100644 index 00000000..089579aa --- /dev/null +++ b/frontend/src/tests/SubmitContributionResubmit.test.js @@ -0,0 +1,399 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/svelte/svelte5'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import SubmitContribution from '../routes/SubmitContribution.svelte'; + +const mocks = vi.hoisted(() => ({ + query: 'resubmit=rejected-42&mission=999&type=998', + push: vi.fn(), + api: { + get: vi.fn(), + post: vi.fn(), + put: vi.fn() + }, + getAllContributionTypes: vi.fn(), + getContributionType: vi.fn(), + getMission: vi.fn(), + getAcceptedProjects: vi.fn(), + getMissions: vi.fn(), + trackEvent: vi.fn(), + authStore: /** @type {import('svelte/store').Writable | null} */ (null) +})); + +vi.mock('svelte-spa-router', () => ({ + push: mocks.push, + querystring: { + subscribe(run) { + run(mocks.query); + return () => {}; + } + } +})); + +vi.mock('../lib/api.js', () => ({ + default: mocks.api, + contributionsAPI: { + getAllContributionTypes: mocks.getAllContributionTypes, + getContributionType: mocks.getContributionType, + getMission: mocks.getMission + }, + submissionsAPI: { + getAcceptedProjects: mocks.getAcceptedProjects + } +})); + +vi.mock('../lib/missionsStore.js', () => ({ + getMissions: mocks.getMissions +})); + +vi.mock('../lib/auth.js', async () => { + const { writable } = await import('svelte/store'); + mocks.authStore = writable({ + isAuthenticated: true, + address: '0xbuilder', + loading: false, + error: null + }); + return { + authState: mocks.authStore + }; +}); + +vi.mock('../lib/userStore.js', async () => { + const { readable } = await import('svelte/store'); + return { + userStore: readable({ + user: { + address: '0xbuilder', + builder: true, + validator: false, + creator: false, + twitter_connection: null, + discord_connection: null, + github_connection: null + }, + loading: false, + error: null + }) + }; +}); + +vi.mock('../lib/analytics.js', () => ({ + getAnalyticsContext: (properties) => properties, + getLifecycleDurationMs: () => 0, + getLifecycleDurations: () => ({}), + markLifecycleTime: () => false, + trackEvent: mocks.trackEvent +})); + +const genericEvidenceType = { + id: 91, + name: 'Other', + slug: 'other', + is_generic: true, + order: 99, + url_patterns: [] +}; + +const builderType = { + id: 7, + name: 'Builder Project', + slug: 'projects', + category: 'builder', + description: 'Build a project.', + is_submittable: true, + is_full: false, + user_weekly_is_full: false, + min_points: 10, + max_points: 100, + accepted_evidence_url_types: [genericEvidenceType], + required_evidence_url_types: [], + required_social_accounts: [], + required_discord_roles: [] +}; + +function rejectedSource(overrides = {}) { + return { + id: 'rejected-42', + state: 'rejected', + contribution_type: builderType.id, + contribution_type_name: builderType.name, + contribution_type_details: builderType, + contribution_date: '2026-07-18T12:00:00Z', + title: 'Original project title', + notes: 'Original project notes', + staff_reply: 'The evidence did not describe the final release.', + mission: null, + project_contribution: null, + evidence_items: [{ + id: 81, + description: 'Release evidence', + url: 'https://example.com/original-release', + url_type: genericEvidenceType + }], + ...overrides + }; +} + +describe('rejected contribution resubmission', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.query = 'resubmit=rejected-42&mission=999&type=998'; + mocks.api.get.mockResolvedValue({ data: rejectedSource() }); + mocks.api.post.mockResolvedValue({ data: { id: 'new-submission-84' } }); + mocks.getAllContributionTypes.mockResolvedValue({ data: [builderType] }); + mocks.getContributionType.mockResolvedValue({ data: builderType }); + mocks.getMission.mockResolvedValue({ data: null }); + mocks.getAcceptedProjects.mockResolvedValue({ data: [] }); + mocks.getMissions.mockResolvedValue([]); + mocks.authStore?.set({ + isAuthenticated: true, + address: '0xbuilder', + loading: false, + error: null + }); + sessionStorage.clear(); + window.grecaptcha = { + render: vi.fn(() => 12), + getResponse: vi.fn(() => 'recaptcha-token'), + reset: vi.fn() + }; + }); + + it('clones every supported field without record ids and creates a new row', async () => { + render(SubmitContribution); + + expect(await screen.findByRole('heading', { name: 'Resubmit contribution' })).toBeTruthy(); + expect(screen.getByText('Original rejection reason')).toBeTruthy(); + const typeInput = await screen.findByDisplayValue('Builder Project'); + await waitFor(() => expect(typeInput.disabled).toBe(false)); + expect(screen.getByDisplayValue('Original project title')).toBeTruthy(); + expect(screen.getByDisplayValue('Original project notes')).toBeTruthy(); + expect(screen.getByDisplayValue('https://example.com/original-release')).toBeTruthy(); + + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit Contribution' })); + + await waitFor(() => expect(mocks.api.post).toHaveBeenCalledTimes(1)); + expect(mocks.api.put).not.toHaveBeenCalled(); + const [path, payload] = mocks.api.post.mock.calls[0]; + expect(path).toBe('/submissions/'); + expect(payload).toMatchObject({ + contribution_type: builderType.id, + contribution_date: '2026-07-18T00:00:00Z', + title: 'Original project title', + notes: 'Original project notes', + recaptcha: 'recaptcha-token', + evidence_items: [{ + description: 'Release evidence', + url: 'https://example.com/original-release' + }] + }); + expect(payload.evidence_items[0]).not.toHaveProperty('id'); + expect(mocks.getMission).not.toHaveBeenCalledWith(999); + expect(mocks.push).toHaveBeenCalledWith( + '/my-submissions?submission=new-submission-84' + ); + expect(sessionStorage.getItem('submissionUpdateSuccess')).toContain( + 'corrected contribution' + ); + }); + + it('keeps cloned content but requires a new selection for an expired mission', async () => { + const expiredMission = { + id: 55, + name: 'Expired build mission', + contribution_type: builderType.id, + is_active: false, + end_date: '2026-01-01T00:00:00Z', + is_full: false + }; + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ mission: { id: 55, name: expiredMission.name } }) + }); + mocks.getMission.mockResolvedValue({ data: expiredMission }); + + render(SubmitContribution); + + expect(await screen.findByRole('heading', { name: 'Resubmit contribution' })).toBeTruthy(); + expect(await screen.findByText(/is no longer accepting submissions/)).toBeTruthy(); + expect(screen.getByText(/copied details are intact/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Resubmit Contribution' })).toBeNull(); + expect(mocks.getMission).toHaveBeenCalledWith(55); + expect(mocks.getMission).not.toHaveBeenCalledWith(999); + }); + + it('preserves cloned content but does not preselect a retired type', async () => { + const retiredType = { + ...builderType, + name: 'Retired Builder Type', + is_submittable: false + }; + const replacementType = { + ...builderType, + id: 8, + name: 'Current Builder Type', + slug: 'current-builder-type' + }; + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ + contribution_type_name: retiredType.name, + contribution_type_details: retiredType + }) + }); + mocks.getAllContributionTypes.mockResolvedValue({ + data: [retiredType, replacementType] + }); + + render(SubmitContribution); + + expect(await screen.findByText(/no longer accepts direct submissions/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Resubmit Contribution' })).toBeNull(); + await fireEvent.click(screen.getByText(replacementType.name)); + expect(screen.getByDisplayValue('Original project title')).toBeTruthy(); + expect(screen.getByDisplayValue('https://example.com/original-release')).toBeTruthy(); + expect(screen.getByRole('button', { name: 'Resubmit Contribution' })).toBeTruthy(); + }); + + it('preserves cloned content but does not preselect a full type', async () => { + const fullType = { + ...builderType, + name: 'Full Builder Type', + is_full: true + }; + const replacementType = { + ...builderType, + id: 8, + name: 'Current Builder Type', + slug: 'current-builder-type' + }; + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ + contribution_type_name: fullType.name, + contribution_type_details: fullType + }) + }); + mocks.getAllContributionTypes.mockResolvedValue({ + data: [fullType, replacementType] + }); + + render(SubmitContribution); + + expect(await screen.findByText(/is currently full/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Resubmit Contribution' })).toBeNull(); + await fireEvent.click(screen.getByText(replacementType.name)); + expect(screen.getByDisplayValue('Original project notes')).toBeTruthy(); + expect(screen.getByRole('button', { name: 'Resubmit Contribution' })).toBeTruthy(); + }); + + it('prefills an active mission and keeps the normal POST path', async () => { + const activeMission = { + id: 55, + name: 'Active build mission', + contribution_type: builderType.id, + is_active: true, + is_full: false, + user_is_full: false + }; + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ mission: { id: 55, name: activeMission.name } }) + }); + mocks.getMissions.mockResolvedValue([activeMission]); + + render(SubmitContribution); + + const missionInput = await screen.findByDisplayValue(activeMission.name); + await waitFor(() => expect(missionInput.disabled).toBe(false)); + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit Contribution' })); + + await waitFor(() => expect(mocks.api.post).toHaveBeenCalledTimes(1)); + expect(mocks.api.post.mock.calls[0][1]).toMatchObject({ + contribution_type: builderType.id, + mission: activeMission.id, + recaptcha: 'recaptcha-token' + }); + }); + + it('prefills an available linked project for a milestone clone', async () => { + const milestoneType = { + ...builderType, + name: 'Milestones', + slug: 'milestones' + }; + const linkedProject = { + id: 301, + title: 'Original highlighted project', + next_milestone_version: 3 + }; + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ + contribution_type_name: milestoneType.name, + contribution_type_details: milestoneType, + project_contribution: linkedProject + }) + }); + mocks.getAllContributionTypes.mockResolvedValue({ data: [milestoneType] }); + mocks.getAcceptedProjects.mockResolvedValue({ data: [linkedProject] }); + + render(SubmitContribution); + + const typeInput = await screen.findByDisplayValue(milestoneType.name); + await waitFor(() => expect(typeInput.disabled).toBe(false)); + await fireEvent.click(screen.getByRole('button', { name: 'Resubmit Contribution' })); + + await waitFor(() => expect(mocks.api.post).toHaveBeenCalledTimes(1)); + expect(mocks.api.post.mock.calls[0][1]).toMatchObject({ + contribution_type: milestoneType.id, + project_contribution: String(linkedProject.id), + recaptcha: 'recaptcha-token' + }); + }); + + it('refuses to prefill a source that is no longer rejected', async () => { + mocks.api.get.mockResolvedValue({ + data: rejectedSource({ state: 'pending' }) + }); + + render(SubmitContribution); + + expect(await screen.findByRole('heading', { name: 'Unable to start resubmission' })).toBeTruthy(); + expect(screen.getByText(/still rejected/)).toBeTruthy(); + expect(mocks.getAllContributionTypes).not.toHaveBeenCalled(); + }); + + it('surfaces source loading failures and supports retry', async () => { + mocks.api.get + .mockRejectedValueOnce(new Error('Network unavailable')) + .mockResolvedValueOnce({ data: rejectedSource() }); + + render(SubmitContribution); + + expect(await screen.findByText(/couldn't load the rejected submission/)).toBeTruthy(); + await fireEvent.click(screen.getByRole('button', { name: 'Try again' })); + + expect(await screen.findByRole('heading', { name: 'Resubmit contribution' })).toBeTruthy(); + expect(mocks.api.get).toHaveBeenCalledTimes(2); + }); + + it('loads the owner-scoped source after signing in without remounting', async () => { + mocks.authStore?.set({ + isAuthenticated: false, + address: null, + loading: false, + error: null + }); + + render(SubmitContribution); + + expect(await screen.findByText('Authentication Required')).toBeTruthy(); + expect(mocks.api.get).not.toHaveBeenCalled(); + + mocks.authStore?.set({ + isAuthenticated: true, + address: '0xbuilder', + loading: false, + error: null + }); + + expect(await screen.findByRole('heading', { name: 'Resubmit contribution' })).toBeTruthy(); + expect(mocks.api.get).toHaveBeenCalledWith('/submissions/rejected-42/'); + }); +}); From eddbe897f745275f8556a3cbf05b846a49d06465 Mon Sep 17 00:00:00 2001 From: JoaquinBN Date: Thu, 6 Aug 2026 13:11:56 +0200 Subject: [PATCH 2/4] Redesign submission guidelines into a compact pre-flight check The guidelines rail on the submit form is now a per-type pre-flight card instead of a long document. A shared ContributionGuidelines component renders panels for Projects, Intelligent Contract, and Milestone plus a minimal default state: the weekly slot counter is the single visual accent and personalizes to the user's remaining slots, the category router can switch the type selector directly, the Milestone panel leads with a highlighted-project eligibility check, and the post-submission process (more information, rejection, appeal) lives in a collapsed accordion so it appears at the right moment. On desktop the card is a sticky right rail with internal scrolling; on mobile it collapses into a disclosure above the form with the slot summary in its header. The Projects contribution-type detail page reuses the same panel. ## Claude Implementation Notes - frontend/src/components/portal/ContributionGuidelines.svelte: Rebuilt as the compact pre-flight card. Per-type panels keyed off slug (projects family / create-intelligent-contracts / milestones / general), personalized slot counter with segment bar and soft warning at 0 remaining (falls back to static copy without user data), clickable category router via the onRoute prop (plain text without it), quality bars per panel, collapsed "After you submit" accordion reset via {#key panel}, and a mobile
variant titled "Before you submit". - frontend/src/lib/contributionGuidelines.js: isProjectContributionType helper centralizing legacy project slug detection. - frontend/src/components/portal/submit-contribution/SubmitContribution.svelte: routeToType(slug) wires the router into selectType; milestoneEligible derived stays null while projects load or errored so transient failures never claim ineligibility; mobile disclosure moved above the form; desktop aside is always sticky with max-height viewport scroll (removed the project-specific non-sticky special case); isProjectType delegates to the shared helper. - frontend/src/routes/ContributionTypeDetail.svelte: Renders the guidelines panel in detail mode (non-interactive router) on the Projects type page. - frontend/src/tests/ContributionGuidelines.test.js: Covers the default state, all three type panels, counter personalization/warning, router callbacks, and both accordions. - frontend/src/tests/MissionBrowsing.test.js: Detail-page guidance assertions updated to the new heading and weekly-window copy. - frontend/CLAUDE.md: Documents the redesigned component, its panels, and props. --- frontend/CLAUDE.md | 1 + .../portal/ContributionGuidelines.svelte | 744 ++++++++++++++++++ .../SubmitContribution.svelte | 69 +- frontend/src/lib/contributionGuidelines.js | 16 + .../src/routes/ContributionTypeDetail.svelte | 7 + .../src/tests/ContributionGuidelines.test.js | 140 ++++ frontend/src/tests/MissionBrowsing.test.js | 18 + 7 files changed, 994 insertions(+), 1 deletion(-) create mode 100644 frontend/src/components/portal/ContributionGuidelines.svelte create mode 100644 frontend/src/lib/contributionGuidelines.js create mode 100644 frontend/src/tests/ContributionGuidelines.test.js diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 58ea4d9a..6c852135 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -590,6 +590,7 @@ Reusable, data-driven display components that accept data via props. Used on Das #### Portal Overview Components (`src/components/portal/`) Investor-oriented home page (`routes/Overview.svelte`), top to bottom: hero → network activity (with portal contributors) → projects (`FeaturedBuilds`) → partner marquee. +- **`ContributionGuidelines.svelte`** - Compact pre-flight submission card shared by the create form and the Projects contribution-type detail page. Per-type panels: Projects (slot counter, category router, quality bar, go-further tip), Intelligent Contract (`create-intelligent-contracts`; quality bar + strict note), Milestone (`milestones`; leads with a highlighted-project eligibility check via the `milestoneEligible` prop), and a minimal default state (weekly-window chip only). The slot counter is the single accent element: personalized ("You have used X of N Project slots this week", segment bar, soft warning at 0 left) when `user_weekly_submissions_remaining` is present, static otherwise. Post-submission process (More information / Rejection / Appeal) lives only in the collapsed "After you submit" accordion inside type panels. The `onRoute(slug)` prop makes the category-router names clickable (the create form wires it to `selectType`); without it they render as plain text (detail page). The create form renders the card as a sticky right rail at `xl` (max-height + internal scroll) and as a collapsed `
` titled "Before you submit" above the form below `xl`, with the slot summary in the collapsed header. Project slug detection is centralized in `lib/contributionGuidelines.js` so historical type URLs keep working. - **`HeroBanner.svelte`** - supports `compact` (thinner banner) and `socialStats={ x, telegram, discord }` (a discreet brand-logo follower-count cluster top-right, Overview only; each badge links out to X/Telegram/Discord). The CTA button reads "View"; a "See all" text link (→ `/gen-news`) sits bottom-right next to the carousel dots. Overview fetches `metricsAPI.getOverview()` for `socialStats`. - **`NetworkActivity.svelte`** - two-column section: LEFT `PortalStats`, RIGHT the network-activity panel (headline KPIs — decisions, chain TXs, DeFiLlama rank — + `DecisionsChart`). Fetches `metricsAPI.getNetworkActivity()`. (The old "Securing the network" validators panel was removed.) - **`PortalStats.svelte`** - "Portal contributors" panel: Builders / Validators / Community members / Contributions in a column, hexagon `CategoryIcon` style. Reads the **public** `metricsAPI.getOverview()` (`metrics.{builders,validators,community_members,contributions}.value`) — NOT `statsAPI.getDashboardStats()`, which is auth-only and would render blank for public visitors. diff --git a/frontend/src/components/portal/ContributionGuidelines.svelte b/frontend/src/components/portal/ContributionGuidelines.svelte new file mode 100644 index 00000000..4597886f --- /dev/null +++ b/frontend/src/components/portal/ContributionGuidelines.svelte @@ -0,0 +1,744 @@ + + +{#snippet checkIcon()} + +{/snippet} + +{#snippet slotCounter()} + {#if weeklyLimit !== null} +
+ {#if slotsUsed !== null} +

+ {#if slotsRemaining === 0} + You have used your {weeklyLimit} {slotNoun} for this week. + {:else} + You have used {slotsUsed} of {weeklyLimit} {slotNoun} this week + {/if} + Resets Monday 00:00 UTC +

+ + {:else} +

+ {weeklyLimit} new {submissionNoun} per user, each week + {WEEK_WINDOW} +

+ {/if} +
+ {/if} +{/snippet} + +{#snippet categoryRouter()} +
+
+
+ A standalone reusable contract + + {#if onRoute} + + {:else} + Intelligent Contract + {/if} +
+
+ An improvement to an accepted project + + {#if onRoute} + + {:else} + Milestone + {/if} +
+
+

+ Milestones are open only for highlighted projects. Cosmetic changes do not qualify. +

+
+{/snippet} + +{#snippet qualityList()} +
+

Quality bar

+
    + {#each qualityBar as item} +
  • + {@render checkIcon()} + {item[0]} {item[1]} +
    • + {/each} +
+ {#if panel === 'contract'} +

Acceptance for this category is strict. Lightweight contracts are rejected.

+ {/if} +
+{/snippet} + +{#snippet afterSubmit()} +
+ + After you submit: reviews, appeals, and slots + + +
+

More information. We see a path to acceptance and need details. Update the existing submission. Does not use a slot.

+

Rejection. The use case or implementation falls short. Fixes or new evidence require a new submission, which uses a slot.

+

Appeal. Challenges the original decision only, with the work as it was submitted. Does not use a slot.

+
+
+{/snippet} + +{#snippet panelBody()} + {#key panel} +
+ {#if panel === 'projects'} + {@render slotCounter()} + {@render categoryRouter()} + {@render qualityList()} +

Go further: live demos, videos, and public posts earn extra points and speed up review.

+ {@render afterSubmit()} + {:else if panel === 'contract'} + {@render slotCounter()} + {@render qualityList()} + {@render afterSubmit()} + {:else if panel === 'milestone'} + {#if milestoneEligible === false} +

+ You have no highlighted Project contributions yet. + Milestones can only be linked to a highlighted Project. +

+ {:else} +

+ Link a highlighted Project contribution, then describe what changed. Cosmetic changes do not qualify. +

+ {/if} + {@render slotCounter()} + {@render afterSubmit()} + {:else if weeklyLimit !== null} + {@render slotCounter()} + {:else} +

+ + Weekly limits run {WEEK_WINDOW} +

+ {/if} +
+ {/key} +{/snippet} + +{#snippet headerIcon()} + +{/snippet} + +{#if mobile} +
+ + + + Before you submit + {mobileSummaryLine} + + + +
+ {#if panel !== 'general'} +

{subtitle}

+ {/if} + {@render panelBody()} +
+
+{:else} +
+
+ +
+

{title}

+

{subtitle}

+
+
+ {@render panelBody()} +
+{/if} + + diff --git a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte index c006e98b..2466e841 100644 --- a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte +++ b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte @@ -6,7 +6,9 @@ import { getMissions } from "../../../lib/missionsStore.js"; import { authState } from "../../../lib/auth.js"; import { userStore } from "../../../lib/userStore"; + import { isProjectContributionType } from "../../../lib/contributionGuidelines.js"; import ConfirmDialog from "../../ConfirmDialog.svelte"; + import ContributionGuidelines from "../ContributionGuidelines.svelte"; import { parseMarkdown } from "../../../lib/markdownLoader.js"; import { getAnalyticsContext, @@ -646,7 +648,7 @@ } function isProjectType(type) { - return ["projects", "projects-and-milestones", "projects-milestones", "project-milestone"].includes(type?.slug); + return isProjectContributionType(type); } async function loadAcceptedProjects() { @@ -667,6 +669,12 @@ } } + // null while loading or after a fetch error so the guidance panel never + // tells a builder they are ineligible off transient state. + let milestoneEligible = $derived( + loadingProjects || projectsError ? null : acceptedProjects.length > 0, + ); + let selectedProjectData = $derived( acceptedProjects.find((project) => String(project.id) === String(selectedProject)) || null ); @@ -833,6 +841,11 @@ showProjectDropdown = false; } + function routeToType(slug) { + const target = types.find((type) => type.slug === slug); + if (target) selectType(target); + } + function selectType(t) { if (selectionLocked) return; if (!isOriginalType(t) && isTypeFull(t)) { @@ -1589,6 +1602,7 @@
{#if editMode} @@ -1679,6 +1693,17 @@

{/if} + {#if !editMode} +
+ +
+ {/if} +
{/if}
+ + {#if !editMode} + + {/if} .submit-page-title { + grid-column: 1; + grid-row: 1; + } + + .submit-form-shell.with-guidelines > form { + grid-column: 1; + grid-row: 2; + min-width: 0; + } + + .desktop-guidelines-slot { + grid-column: 2; + grid-row: 2; + max-height: calc(100vh - 48px); + min-width: 0; + overflow-y: auto; + position: sticky; + top: 24px; + } + } + .submit-panel { border-color: rgba(0, 0, 0, 0.055); box-shadow: diff --git a/frontend/src/lib/contributionGuidelines.js b/frontend/src/lib/contributionGuidelines.js new file mode 100644 index 00000000..076c9399 --- /dev/null +++ b/frontend/src/lib/contributionGuidelines.js @@ -0,0 +1,16 @@ +const PROJECT_CONTRIBUTION_TYPE_SLUGS = new Set([ + 'projects', + 'projects-and-milestones', + 'projects-milestones', + 'project-milestone', +]); + +/** + * Keep legacy project slugs recognizable while old contribution types remain + * addressable from historical submissions. + * + * @param {{ slug?: string } | null | undefined} contributionType + */ +export function isProjectContributionType(contributionType) { + return PROJECT_CONTRIBUTION_TYPE_SLUGS.has(contributionType?.slug); +} diff --git a/frontend/src/routes/ContributionTypeDetail.svelte b/frontend/src/routes/ContributionTypeDetail.svelte index 0a918c21..74807a47 100644 --- a/frontend/src/routes/ContributionTypeDetail.svelte +++ b/frontend/src/routes/ContributionTypeDetail.svelte @@ -8,7 +8,9 @@ import { getMissions } from '../lib/missionsStore.js'; import HighlightsSlider from '../components/portal/HighlightsSlider.svelte'; import PortalContributionCard from '../components/portal/PortalContributionCard.svelte'; + import ContributionGuidelines from '../components/portal/ContributionGuidelines.svelte'; import { getCategoryButtonStyle, getCategoryGradientStyle } from '../lib/categoryPresentation.js'; + import { isProjectContributionType } from '../lib/contributionGuidelines.js'; import { visibleContributions } from '../lib/hiddenContributions.js'; import { userStore } from '../lib/userStore.js'; import { hasReadOnlyRoleSectionAccess } from '../lib/roleState.js'; @@ -73,6 +75,7 @@ let allContributionsPath = $derived( `/all-contributions?category=${explorerCategory}&type=${params.id}` ); + let hasProjectGuidelines = $derived(isProjectContributionType(contributionType)); function formatDate(dateString) { if (!dateString) return 'Never'; @@ -324,6 +327,10 @@ {/if} + {#if hasProjectGuidelines} + + {/if} + {#if missions.length > 0}
diff --git a/frontend/src/tests/ContributionGuidelines.test.js b/frontend/src/tests/ContributionGuidelines.test.js new file mode 100644 index 00000000..89808ec3 --- /dev/null +++ b/frontend/src/tests/ContributionGuidelines.test.js @@ -0,0 +1,140 @@ +import { fireEvent, render, screen } from '@testing-library/svelte/svelte5'; +import { describe, expect, it, vi } from 'vitest'; +import ContributionGuidelines from '../components/portal/ContributionGuidelines.svelte'; + +const projectType = { + id: 7, + name: 'Projects', + slug: 'projects', + max_submissions_per_user_per_week: 2, +}; + +describe('ContributionGuidelines', () => { + it('shows only a minimal pre-flight card before a contribution type is selected', () => { + render(ContributionGuidelines); + + expect(screen.getByRole('heading', { name: 'Before you submit' })).toBeTruthy(); + expect(screen.getByText('Weekly limits run Monday 00:00 to Sunday 23:59 UTC')).toBeTruthy(); + expect(screen.queryByText('Quality bar')).toBeNull(); + expect(screen.queryByText('After you submit: reviews, appeals, and slots')).toBeNull(); + expect(screen.queryByText('More information.', { exact: false })).toBeNull(); + }); + + it('shows the Project panel with the static limit when no personal usage data exists', () => { + render(ContributionGuidelines, { props: { contributionType: projectType } }); + + expect(screen.getByRole('heading', { name: 'Before you submit a Project' })).toBeTruthy(); + expect(screen.getByText('2 new Project submissions per user, each week')).toBeTruthy(); + expect(screen.getByText('Monday 00:00 to Sunday 23:59 UTC')).toBeTruthy(); + expect(screen.getByText('Quality bar')).toBeTruthy(); + expect(screen.getByText('Solves a real trust problem.')).toBeTruthy(); + expect( + screen.getByText('Milestones are open only for highlighted projects. Cosmetic changes do not qualify.'), + ).toBeTruthy(); + }); + + it('personalizes the slot counter when weekly usage data is available', () => { + render(ContributionGuidelines, { + props: { + contributionType: { ...projectType, user_weekly_submissions_remaining: 1 }, + }, + }); + + expect(screen.getByText('You have used 1 of 2 Project slots this week')).toBeTruthy(); + expect(screen.getByText('Resets Monday 00:00 UTC')).toBeTruthy(); + expect(screen.getByRole('img', { name: '1 of 2 weekly slots used' })).toBeTruthy(); + }); + + it('switches the counter to a warning when no weekly slots remain', () => { + render(ContributionGuidelines, { + props: { + contributionType: { ...projectType, user_weekly_submissions_remaining: 0 }, + }, + }); + + expect(screen.getByText('You have used your 2 Project slots for this week.')).toBeTruthy(); + expect(screen.getByText('Resets Monday 00:00 UTC')).toBeTruthy(); + }); + + it('routes to the sibling categories when the router entries are clicked', async () => { + const onRoute = vi.fn(); + render(ContributionGuidelines, { + props: { contributionType: projectType, onRoute }, + }); + + await fireEvent.click(screen.getByRole('button', { name: 'Intelligent Contract' })); + expect(onRoute).toHaveBeenCalledWith('create-intelligent-contracts'); + + await fireEvent.click(screen.getByRole('button', { name: 'Milestone' })); + expect(onRoute).toHaveBeenCalledWith('milestones'); + }); + + it('renders the router entries as plain text without an onRoute callback', () => { + render(ContributionGuidelines, { props: { contributionType: projectType } }); + + expect(screen.queryByRole('button', { name: 'Milestone' })).toBeNull(); + expect(screen.getByText('Milestone')).toBeTruthy(); + }); + + it('keeps the post-submission process collapsed behind an accordion', async () => { + render(ContributionGuidelines, { props: { contributionType: projectType } }); + + const accordion = screen + .getByText('After you submit: reviews, appeals, and slots') + .closest('details'); + expect(accordion.open).toBe(false); + + await fireEvent.click(screen.getByText('After you submit: reviews, appeals, and slots')); + expect(accordion.open).toBe(true); + expect(screen.getByText('More information.')).toBeTruthy(); + expect(screen.getByText('Rejection.')).toBeTruthy(); + expect(screen.getByText('Appeal.')).toBeTruthy(); + }); + + it('shows the Intelligent Contract quality bar for the contract type', () => { + render(ContributionGuidelines, { + props: { + contributionType: { id: 9, name: 'Intelligent Contract', slug: 'create-intelligent-contracts' }, + }, + }); + + expect( + screen.getByRole('heading', { name: 'Before you submit an Intelligent Contract' }), + ).toBeTruthy(); + expect(screen.getByText('Not a learning exercise.')).toBeTruthy(); + expect( + screen.getByText('Acceptance for this category is strict. Lightweight contracts are rejected.'), + ).toBeTruthy(); + expect(screen.queryByText('Solves a real trust problem.')).toBeNull(); + }); + + it('leads the Milestone panel with an eligibility warning when the user has no highlighted projects', () => { + render(ContributionGuidelines, { + props: { + contributionType: { id: 10, name: 'Milestones', slug: 'milestones' }, + milestoneEligible: false, + }, + }); + + expect(screen.getByRole('heading', { name: 'Before you submit a Milestone' })).toBeTruthy(); + expect(screen.getByText('You have no highlighted Project contributions yet.')).toBeTruthy(); + }); + + it('renders the mobile guidance as a collapsed accordion with the slot counter in the header', async () => { + render(ContributionGuidelines, { + props: { + contributionType: { ...projectType, user_weekly_submissions_remaining: 1 }, + mobile: true, + }, + }); + + const disclosure = screen.getByText('Before you submit').closest('details'); + expect(disclosure).toBeTruthy(); + expect(disclosure.open).toBe(false); + expect(screen.getByText('1 of 2 Project slots used this week')).toBeTruthy(); + + await fireEvent.click(screen.getByText('Before you submit')); + expect(disclosure.open).toBe(true); + expect(screen.getByText('Quality bar')).toBeTruthy(); + }); +}); diff --git a/frontend/src/tests/MissionBrowsing.test.js b/frontend/src/tests/MissionBrowsing.test.js index 4db084c2..283d08a8 100644 --- a/frontend/src/tests/MissionBrowsing.test.js +++ b/frontend/src/tests/MissionBrowsing.test.js @@ -129,6 +129,24 @@ describe('historical mission browsing', () => { expect(mocks.push).toHaveBeenCalledWith('/mission/12'); }); + it('shows the submission guidance panel on the Projects type page', async () => { + mocks.getContributionType.mockResolvedValue({ + data: { + ...contributionType, + name: 'Projects', + slug: 'projects', + max_submissions_per_user_per_week: 2, + }, + }); + + render(ContributionTypeDetail, { props: { params: { id: '7' } } }); + + expect( + await screen.findByRole('heading', { name: 'Before you submit a Project' }), + ).toBeTruthy(); + expect(screen.getByText('Monday 00:00 to Sunday 23:59 UTC')).toBeTruthy(); + }); + it('offers ended missions as filters in all contributions', async () => { window.history.replaceState({}, '', '/all-contributions?type=7&mission=12'); render(AllContributions); From 91aaf6ae3545802393ac22723fb649bbb5085605 Mon Sep 17 00:00:00 2001 From: JoaquinBN Date: Thu, 6 Aug 2026 13:12:08 +0200 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77164813..0030f15e 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 +- The Submit Contribution page now shows a compact "Before you submit" guidance card that adapts to the selected type: a personal weekly slot counter for Projects, quality bars for Projects and Intelligent Contracts, a milestone eligibility check, one-click switching between related types, and review/appeal rules tucked behind a collapsed accordion (also shown on the Projects type page) (eddbe897) + - 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) From b349cfc82770e81796783378fea0178d7ab1f7f3 Mon Sep 17 00:00:00 2001 From: JoaquinBN Date: Thu, 6 Aug 2026 13:25:03 +0200 Subject: [PATCH 4/4] Address resubmission review feedback --- .../tests/test_discord_role_gating.py | 41 +++++++++- backend/contributions/views.py | 81 ++++++++++++++++++- frontend/src/components/SubmissionCard.svelte | 25 ++---- .../SubmitContribution.svelte | 40 +++------ frontend/src/lib/api.js | 21 +---- frontend/src/lib/submissionErrors.js | 61 ++++++++++++++ frontend/src/routes/MySubmissions.svelte | 41 +++++----- frontend/src/tests/submissionErrors.test.js | 40 +++++++++ 8 files changed, 256 insertions(+), 94 deletions(-) create mode 100644 frontend/src/lib/submissionErrors.js create mode 100644 frontend/src/tests/submissionErrors.test.js diff --git a/backend/contributions/tests/test_discord_role_gating.py b/backend/contributions/tests/test_discord_role_gating.py index b8d085d4..7b0f9c2c 100644 --- a/backend/contributions/tests/test_discord_role_gating.py +++ b/backend/contributions/tests/test_discord_role_gating.py @@ -1,6 +1,8 @@ +from types import SimpleNamespace from unittest.mock import patch from django.contrib.auth import get_user_model +from django.db import transaction from django.test import TestCase, override_settings from django.utils import timezone from rest_framework import status @@ -20,7 +22,7 @@ class DiscordRoleSubmissionGatingTest(TestCase): def setUp(self): self.category = Category.objects.create( - name='Community', + name='Discord Role Test', slug='community-test', description='Community test category', ) @@ -170,3 +172,40 @@ def test_refresh_failure_fails_closed(self, mock_sync_member_roles): self.assertEqual(response.status_code, status.HTTP_503_SERVICE_UNAVAILABLE) self.assertIn('temporarily unavailable', response.data['error']) + + @patch('social_connections.discord_roles.DiscordRoleSyncService.sync_member_roles') + def test_edit_refreshes_roles_before_locking_submission(self, mock_sync_member_roles): + connection = DiscordConnection.objects.create( + user=self.user, + platform_user_id='discord-user', + platform_username='discorduser', + linked_at=timezone.now(), + ) + submission = self._create_pending_submission(self.contribution_type) + baseline_savepoints = list(transaction.get_connection().savepoint_ids) + + def sync_roles(discord_connection): + self.assertEqual( + list(transaction.get_connection().savepoint_ids), + baseline_savepoints, + ) + discord_connection.guild_member = True + discord_connection.roles_synced_at = timezone.now() + discord_connection.save( + update_fields=['guild_member', 'roles_synced_at'], + ) + discord_connection.current_roles.add(self.required_role) + return SimpleNamespace(connection=discord_connection) + + mock_sync_member_roles.side_effect = sync_roles + + response = self.client.patch( + f'/api/v1/submissions/{submission.id}/', + {'notes': 'Updated after Discord refresh'}, + format='json', + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK, response.data) + mock_sync_member_roles.assert_called_once_with(connection) + submission.refresh_from_db() + self.assertEqual(submission.notes, 'Updated after Discord refresh') diff --git a/backend/contributions/views.py b/backend/contributions/views.py index 49073503..5c768805 100644 --- a/backend/contributions/views.py +++ b/backend/contributions/views.py @@ -620,7 +620,13 @@ def get_serializer_context(self): context['use_light_serializers'] = self.action == 'list' return context - def _validate_required_discord_roles(self, user, contribution_type): + def _validate_required_discord_roles( + self, + user, + contribution_type, + *, + sync_if_stale=True, + ): """Ensure the user has at least one role required by this type.""" required_roles = list( contribution_type.required_discord_roles.filter( @@ -646,7 +652,7 @@ def _validate_required_discord_roles(self, user, contribution_type): <= int(getattr(settings, 'DISCORD_ROLE_SUBMISSION_SYNC_GRACE_SECONDS', 30)) ) - if not recently_synced: + if not recently_synced and sync_if_stale: try: from social_connections.discord_roles import ( DiscordRoleSyncConfigurationError, @@ -704,6 +710,7 @@ def _validate_submission_contribution_type( weekly_capacity_at=None, allow_existing_community_submission=False, skip_submittable_check=False, + sync_discord_roles=True, ): """Validate role, category, and requirement gates for submissions.""" if mission and contribution_type.id != mission.contribution_type_id: @@ -771,12 +778,57 @@ def _validate_submission_contribution_type( status=status.HTTP_403_FORBIDDEN ) - discord_role_error = self._validate_required_discord_roles(user, contribution_type) + discord_role_error = self._validate_required_discord_roles( + user, + contribution_type, + sync_if_stale=sync_discord_roles, + ) if discord_role_error is not None: return discord_role_error return None + def _preflight_update_discord_roles(self, request, submission_id): + """Refresh Discord roles before the submission row is locked.""" + submission = ( + SubmittedContribution.objects + .filter(id=submission_id, user=request.user) + .only('contribution_type_id', 'state', 'has_appeal') + .first() + ) + if not submission or submission.state not in ['pending', 'more_info_needed']: + return None, None + if submission.has_appeal and submission.state == 'pending': + return None, None + + requested_type_id = request.data.get( + 'contribution_type', + submission.contribution_type_id, + ) + try: + requested_type_id = int(requested_type_id) + except (TypeError, ValueError): + # The serializer will return the canonical field error. + return None, None + + contribution_type = ( + ContributionType.objects + .select_related('category') + .prefetch_related('required_discord_roles') + .filter(id=requested_type_id) + .first() + ) + if not contribution_type: + return None, None + + return ( + self._validate_required_discord_roles( + request.user, + contribution_type, + ), + contribution_type.id, + ) + def _project_link_error( self, user, @@ -975,6 +1027,13 @@ def create(self, request, *args, **kwargs): def update(self, request, *args, **kwargs): """Update submission (only allowed if state is 'pending' or 'more_info_needed').""" partial = kwargs.pop('partial', False) + ( + discord_role_error, + preflight_contribution_type_id, + ) = self._preflight_update_discord_roles(request, kwargs.get('pk')) + if discord_role_error is not None: + return discord_role_error + with transaction.atomic(): # Serialize all edits/resubmissions for a submission. In particular, # two clicks on Resubmit must never create two responses for one @@ -1123,6 +1182,19 @@ def update(self, request, *args, **kwargs): .prefetch_related('required_discord_roles') .get(id=contribution_type.id) ) + if ( + preflight_contribution_type_id is not None + and contribution_type.id != preflight_contribution_type_id + ): + return Response( + { + 'error': ( + 'The submission changed while it was being edited. ' + 'Refresh and try again.' + ), + }, + status=status.HTTP_409_CONFLICT, + ) keeps_same_contribution_type = ( contribution_type.id == instance.contribution_type_id ) @@ -1141,6 +1213,9 @@ def update(self, request, *args, **kwargs): # A type made non-submittable after the fact must not lock users # out of editing their existing pending/more-info submissions. skip_submittable_check=keeps_same_contribution_type, + # Any outbound refresh happened before acquiring the row lock. + # Re-check only the freshly cached membership and roles here. + sync_discord_roles=False, ) if contribution_type_error is not None: return contribution_type_error diff --git a/frontend/src/components/SubmissionCard.svelte b/frontend/src/components/SubmissionCard.svelte index 8fa7f114..c6b7bafd 100644 --- a/frontend/src/components/SubmissionCard.svelte +++ b/frontend/src/components/SubmissionCard.svelte @@ -2,6 +2,7 @@ import { push } from 'svelte-spa-router'; import { format } from '../lib/dates.js'; import { parseMarkdown, parseUserMarkdown } from '../lib/markdownLoader.js'; + import { submissionErrorMessage } from '../lib/submissionErrors.js'; import { showError, showSuccess } from '../lib/toastStore.js'; import Badge from './Badge.svelte'; import ContributionCard from './ContributionCard.svelte'; @@ -149,25 +150,6 @@ } } - /** @param {any} error */ - function moreInfoErrorMessage(error) { - const responseData = error?.response?.data || {}; - const responseError = responseData.more_info_response; - if (typeof responseError === 'string') return responseError; - if (Array.isArray(responseError)) return responseError[0]; - if (responseError && typeof responseError === 'object') { - const firstValue = Object.values(responseError)[0]; - if (Array.isArray(firstValue)) return firstValue[0]; - if (typeof firstValue === 'string') return firstValue; - } - const evidenceError = responseData.evidence_items; - if (typeof evidenceError === 'string') return evidenceError; - if (Array.isArray(evidenceError) && evidenceError.length > 0) { - return evidenceError[0]?.message || String(evidenceError[0]); - } - return responseData.error || responseData.detail || 'We could not resubmit this contribution. Open Edit to review the current requirements.'; - } - async function handleMoreInfoResubmit() { const message = moreInfoResponse.trim(); if (!onResubmit || !message || resubmitting) return; @@ -182,7 +164,10 @@ moreInfoResponse = ''; showingResubmitResponse = false; } catch (error) { - resubmitError = moreInfoErrorMessage(error); + resubmitError = submissionErrorMessage( + error, + 'We could not resubmit this contribution. Open Edit to review the current requirements.' + ); } finally { resubmitting = false; } diff --git a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte index 9c2c9479..3f9318d4 100644 --- a/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte +++ b/frontend/src/components/portal/submit-contribution/SubmitContribution.svelte @@ -5,6 +5,7 @@ import { contributionsAPI, submissionsAPI } from "../../../lib/api.js"; import { getMissions } from "../../../lib/missionsStore.js"; import { authState } from "../../../lib/auth.js"; + import { submissionErrorMessage } from "../../../lib/submissionErrors.js"; import { userStore } from "../../../lib/userStore"; import ConfirmDialog from "../../ConfirmDialog.svelte"; import { parseMarkdown } from "../../../lib/markdownLoader.js"; @@ -1689,34 +1690,13 @@ error = Array.isArray(err.response.data.recaptcha) ? err.response.data.recaptcha[0] : err.response.data.recaptcha; - } else if (err.response?.data?.evidence_items) { - // Parse evidence validation errors from backend - const evidenceErrors = err.response.data.evidence_items; - if (Array.isArray(evidenceErrors) && evidenceErrors.length > 0) { - const first = evidenceErrors[0]; - error = first.message || JSON.stringify(first); - } else { - error = typeof evidenceErrors === "string" - ? evidenceErrors - : JSON.stringify(evidenceErrors); - } - } else if (err.response?.data?.more_info_response) { - const responseError = err.response.data.more_info_response; - if (typeof responseError === "string") { - error = responseError; - } else if (Array.isArray(responseError)) { - error = responseError[0]; - } else { - const firstValue = Object.values(responseError)[0]; - error = Array.isArray(firstValue) ? firstValue[0] : firstValue; - } } else { - error = - err.response?.data?.error || - err.response?.data?.detail || - (editMode + error = submissionErrorMessage( + err, + editMode ? "Failed to save submission" - : "Failed to submit contribution"); + : "Failed to submit contribution", + ); } if (!editMode && recaptchaWidgetId !== null && window.grecaptcha) { @@ -1892,7 +1872,7 @@
-

+

Corrected submission

-

+

Resubmit contribution

-

+

Review and update this copied content before creating a new submission. The rejected original and its appeal history remain unchanged.

diff --git a/frontend/src/lib/api.js b/frontend/src/lib/api.js index 10f1f215..8ad8371a 100644 --- a/frontend/src/lib/api.js +++ b/frontend/src/lib/api.js @@ -188,28 +188,11 @@ export const submissionsAPI = { appeal: (id, reason) => api.post(`/submissions/${id}/appeal/`, { reason }), /** * @param {string | number} id - * @param {Record} submission * @param {{ request_id: number | null, message: string }} response */ - respondToMoreInfo: (id, submission, response) => api.put( + respondToMoreInfo: (id, response) => api.patch( `/submissions/${id}/`, - { - contribution_type: submission.contribution_type, - contribution_date: submission.contribution_date, - title: submission.title || '', - notes: submission.notes || '', - mission: submission.mission?.id ?? submission.mission ?? null, - project_contribution: - submission.project_contribution?.id ?? - submission.project_contribution ?? - null, - evidence_items: (submission.evidence_items || []).map((evidence) => ({ - ...(evidence.id ? { id: evidence.id } : {}), - description: evidence.description || '', - url: evidence.url || '' - })), - more_info_response: response - } + { more_info_response: response } ), /** @param {string | null} submissionId */ getAcceptedProjects: (submissionId = null) => diff --git a/frontend/src/lib/submissionErrors.js b/frontend/src/lib/submissionErrors.js new file mode 100644 index 00000000..63076f09 --- /dev/null +++ b/frontend/src/lib/submissionErrors.js @@ -0,0 +1,61 @@ +/** + * Return the first useful message from DRF's nested validation-error shapes. + * + * @param {unknown} value + * @returns {string} + */ +function firstErrorMessage(value) { + if (typeof value === 'string') return value.trim(); + + if (Array.isArray(value)) { + for (const item of value) { + const message = firstErrorMessage(item); + if (message) return message; + } + return ''; + } + + if (value && typeof value === 'object') { + const record = /** @type {Record} */ (value); + if (typeof record.message === 'string') { + const message = record.message.trim(); + if (message) return message; + } + + for (const nestedValue of Object.values(record)) { + const message = firstErrorMessage(nestedValue); + if (message) return message; + } + } + + return ''; +} + +/** + * Extract a submitter-facing message from an API or network error. + * + * @param {any} error + * @param {string} fallback + * @returns {string} + */ +export function submissionErrorMessage(error, fallback) { + const responseData = error?.response?.data || {}; + const fields = [ + responseData.more_info_response, + responseData.evidence_items, + responseData.error, + responseData.detail, + ]; + + for (const field of fields) { + const message = firstErrorMessage(field); + if (message) return message; + } + + if (!error?.response && typeof error?.message === 'string') { + const message = error.message.trim(); + if (message) return message; + } + + return fallback; +} diff --git a/frontend/src/routes/MySubmissions.svelte b/frontend/src/routes/MySubmissions.svelte index 9c6b96f7..d71fc4b2 100644 --- a/frontend/src/routes/MySubmissions.svelte +++ b/frontend/src/routes/MySubmissions.svelte @@ -265,19 +265,28 @@ push('/submit-contribution'); } + /** + * @param {string | number} submissionId + * @param {Record} updatedSubmission + */ + function reconcileUpdatedSubmission(submissionId, updatedSubmission) { + const idx = submissions.findIndex(s => s.id === submissionId); + if (idx === -1) return; + + if (stateFilter && stateFilter !== updatedSubmission.state) { + submissions = submissions.filter(s => s.id !== submissionId); + totalCount = Math.max(0, totalCount - 1); + return; + } + + submissions[idx] = updatedSubmission; + submissions = [...submissions]; + } + async function handleAppeal(submissionId, reason) { try { const response = await submissionsAPI.appeal(submissionId, reason); - const idx = submissions.findIndex(s => s.id === submissionId); - if (idx !== -1) { - if (stateFilter && stateFilter !== response.data.state) { - submissions = submissions.filter(s => s.id !== submissionId); - totalCount = Math.max(0, totalCount - 1); - } else { - submissions[idx] = response.data; - submissions = [...submissions]; - } - } + reconcileUpdatedSubmission(submissionId, response.data); showSuccess('Appeal submitted. A steward will re-review your submission.'); } catch (err) { showError(err.response?.data?.error || 'Failed to submit appeal'); @@ -297,19 +306,9 @@ const response = await submissionsAPI.respondToMoreInfo( submissionId, - submission, moreInfoResponse ); - const idx = submissions.findIndex(s => s.id === submissionId); - if (idx !== -1) { - if (stateFilter && stateFilter !== response.data.state) { - submissions = submissions.filter(s => s.id !== submissionId); - totalCount = Math.max(0, totalCount - 1); - } else { - submissions[idx] = response.data; - submissions = [...submissions]; - } - } + reconcileUpdatedSubmission(submissionId, response.data); showSuccess('Your response was sent and the submission is back in review.'); return response.data; } diff --git a/frontend/src/tests/submissionErrors.test.js b/frontend/src/tests/submissionErrors.test.js new file mode 100644 index 00000000..c3699245 --- /dev/null +++ b/frontend/src/tests/submissionErrors.test.js @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { submissionErrorMessage } from '../lib/submissionErrors.js'; + +describe('submissionErrorMessage', () => { + it('reads nested DRF response and evidence errors', () => { + expect(submissionErrorMessage({ + response: { + data: { + more_info_response: { + request_id: ['Refresh and answer the latest request.'] + } + } + } + }, 'Fallback')).toBe('Refresh and answer the latest request.'); + + expect(submissionErrorMessage({ + response: { + data: { + evidence_items: [{ url: ['Use an accepted evidence URL.'] }] + } + } + }, 'Fallback')).toBe('Use an accepted evidence URL.'); + }); + + it('preserves plain errors and falls back for empty response objects', () => { + expect(submissionErrorMessage( + new Error('Submission is no longer visible.'), + 'Fallback' + )).toBe('Submission is no longer visible.'); + + expect(submissionErrorMessage({ + response: { + data: { + more_info_response: {}, + evidence_items: [] + } + } + }, 'Fallback')).toBe('Fallback'); + }); +});