diff --git a/backend/stewards/admin.py b/backend/stewards/admin.py index ed605aa7..50ef2117 100644 --- a/backend/stewards/admin.py +++ b/backend/stewards/admin.py @@ -63,7 +63,7 @@ class StewardPermissionAdmin(admin.ModelAdmin): @admin.register(FeatureCandidateScore) class FeatureCandidateScoreAdmin(admin.ModelAdmin): - list_display = ('submission', 'steward', 'score', 'created_at', 'updated_at') + list_display = ('submission', 'steward', 'score', 'reason_preview', 'created_at', 'updated_at') list_filter = ('score', 'created_at', 'updated_at') search_fields = ( 'submission__title', @@ -76,6 +76,10 @@ class FeatureCandidateScoreAdmin(admin.ModelAdmin): readonly_fields = ('created_at', 'updated_at') ordering = ('-updated_at',) + def reason_preview(self, obj): + return obj.reason[:80] + '...' if len(obj.reason) > 80 else obj.reason + reason_preview.short_description = 'Reason' + @admin.register(ReviewTemplate) class ReviewTemplateAdmin(admin.ModelAdmin): diff --git a/backend/stewards/migrations/0012_feature_candidate_score_reason.py b/backend/stewards/migrations/0012_feature_candidate_score_reason.py new file mode 100644 index 00000000..9fac39dc --- /dev/null +++ b/backend/stewards/migrations/0012_feature_candidate_score_reason.py @@ -0,0 +1,16 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('stewards', '0011_feature_candidate_scoring'), + ] + + operations = [ + migrations.AddField( + model_name='featurecandidatescore', + name='reason', + field=models.TextField(blank=True, default='', help_text='Reviewer note explaining what stood out for this score.', max_length=2000), + ), + ] diff --git a/backend/stewards/models.py b/backend/stewards/models.py index c1959241..39b53540 100644 --- a/backend/stewards/models.py +++ b/backend/stewards/models.py @@ -42,6 +42,12 @@ class FeatureCandidateScore(BaseModel): validators=[MinValueValidator(0), MaxValueValidator(3)], help_text="0 = not interesting, 1 = weak, 2 = good, 3 = strong.", ) + reason = models.TextField( + max_length=2000, + blank=True, + default='', + help_text="Reviewer note explaining what stood out for this score.", + ) class Meta: unique_together = ['submission', 'steward'] diff --git a/backend/stewards/serializers.py b/backend/stewards/serializers.py index ad974a90..9f55ed63 100644 --- a/backend/stewards/serializers.py +++ b/backend/stewards/serializers.py @@ -81,13 +81,18 @@ class FeatureCandidateSubmissionSerializer(serializers.Serializer): created_at = serializers.DateTimeField(read_only=True) updated_at = serializers.DateTimeField(read_only=True) own_score = serializers.SerializerMethodField() + own_score_reason = serializers.SerializerMethodField() user_details = serializers.SerializerMethodField() contribution_type_details = serializers.SerializerMethodField() evidence_items = serializers.SerializerMethodField() def get_own_score(self, obj): - score_map = self.context.get('own_score_map', {}) - return score_map.get(obj.id) + review_map = self.context.get('own_review_map', {}) + return (review_map.get(obj.id) or {}).get('score') + + def get_own_score_reason(self, obj): + review_map = self.context.get('own_review_map', {}) + return (review_map.get(obj.id) or {}).get('reason') or '' def get_user_details(self, obj): return feature_candidate_user_data(obj.user) diff --git a/backend/stewards/tests.py b/backend/stewards/tests.py index b82d10f8..4c682833 100644 --- a/backend/stewards/tests.py +++ b/backend/stewards/tests.py @@ -193,6 +193,7 @@ def test_reviewer_list_is_blind_and_only_includes_interesting_accepted_projects( self.assertEqual(row['state'], 'accepted') self.assertEqual(row['contribution_type_details']['slug'], 'projects') self.assertIsNone(row['own_score']) + self.assertEqual(row['own_score_reason'], '') self.assertNotIn('id', row['user_details']) self.assertEqual(row['user_details']['address'], self.submitter.address) self.assertEqual(row['user_details']['github_connection'], {'platform_username': 'submitter-gh'}) @@ -220,21 +221,29 @@ def test_reviewer_can_create_and_edit_own_score(self): create_response = self.client.post( f'/api/v1/stewards/feature-reviews/{self.submission.id}/score/', - {'score': 2}, + {'score': 2, 'reason': 'Strong ecosystem fit, but the demo needs polish.'}, ) edit_response = self.client.post( f'/api/v1/stewards/feature-reviews/{self.submission.id}/score/', - {'score': 3}, + {'score': 3, 'reason': 'The contract design and working demo stood out.'}, ) self.assertEqual(create_response.status_code, status.HTTP_200_OK) + self.assertEqual(create_response.data['reason'], 'Strong ecosystem fit, but the demo needs polish.') self.assertEqual(edit_response.status_code, status.HTTP_200_OK) scores = FeatureCandidateScore.objects.filter( submission=self.submission, steward=self.reviewer_steward, ) self.assertEqual(scores.count(), 1) - self.assertEqual(scores.get().score, 3) + score = scores.get() + self.assertEqual(score.score, 3) + self.assertEqual(score.reason, 'The contract design and working demo stood out.') + + list_response = self.client.get('/api/v1/stewards/feature-reviews/') + row = list_response.data['results'][0] + self.assertEqual(row['own_score'], 3) + self.assertEqual(row['own_score_reason'], 'The contract design and working demo stood out.') def test_unpermitted_steward_cannot_score(self): self.client.force_authenticate(user=self.denied_user) @@ -264,6 +273,15 @@ def test_reviewer_score_validation_rejects_invalid_values(self): self.assertIn('score', response.data) self.assertFalse(FeatureCandidateScore.objects.exists()) + response = self.client.post( + url, + {'score': 2, 'reason': 'x' * 2001}, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn('reason', response.data) + self.assertFalse(FeatureCandidateScore.objects.exists()) + def test_staff_admin_sees_aggregates_and_manual_review_flag(self): second_user = User.objects.create_user( email='second-reviewer@example.com', diff --git a/backend/stewards/views.py b/backend/stewards/views.py index 02897537..1f389fc8 100644 --- a/backend/stewards/views.py +++ b/backend/stewards/views.py @@ -69,17 +69,21 @@ def _interesting_queryset(self, include_scores=False): queryset = queryset.prefetch_related('feature_candidate_scores') return queryset - def _own_score_map(self, request): + def _own_review_map(self, request): if not hasattr(request.user, 'steward'): return {} - return dict( - FeatureCandidateScore.objects.filter( + return { + review['submission_id']: { + 'score': review['score'], + 'reason': review['reason'] or '', + } + for review in FeatureCandidateScore.objects.filter( steward=request.user.steward, submission__is_interesting=True, submission__state='accepted', submission__contribution_type__slug='projects', - ).values_list('submission_id', 'score') - ) + ).values('submission_id', 'score', 'reason') + } def _summary_map(self, submissions): result = {} @@ -106,13 +110,13 @@ def list(self, request): ) submissions = list(self._interesting_queryset()) - own_score_map = self._own_score_map(request) + own_review_map = self._own_review_map(request) serializer = FeatureCandidateSubmissionSerializer( submissions, many=True, - context={'own_score_map': own_score_map}, + context={'own_review_map': own_review_map}, ) - scored_count = sum(1 for submission in submissions if submission.id in own_score_map) + scored_count = sum(1 for submission in submissions if submission.id in own_review_map) return Response({ 'results': serializer.data, 'progress': { @@ -129,21 +133,35 @@ def score(self, request, pk=None): status=status.HTTP_403_FORBIDDEN, ) + errors = {} try: score = serializers.IntegerField(min_value=0, max_value=3).run_validation( request.data.get('score') ) except serializers.ValidationError as exc: - return Response({'score': exc.detail}, status=status.HTTP_400_BAD_REQUEST) + errors['score'] = exc.detail + + try: + reason = serializers.CharField( + required=False, + allow_blank=True, + trim_whitespace=True, + max_length=2000, + ).run_validation(request.data.get('reason', '')) + except serializers.ValidationError as exc: + errors['reason'] = exc.detail + + if errors: + return Response(errors, status=status.HTTP_400_BAD_REQUEST) submission = get_object_or_404(self._interesting_queryset(), pk=pk) FeatureCandidateScore.objects.update_or_create( submission=submission, steward=request.user.steward, - defaults={'score': score}, + defaults={'score': score, 'reason': reason}, ) - return Response({'submission': str(submission.id), 'score': score}) + return Response({'submission': str(submission.id), 'score': score, 'reason': reason}) @action(detail=False, methods=['get'], url_path='admin') def admin(self, request): @@ -154,7 +172,7 @@ def admin(self, request): ) submissions = list(self._interesting_queryset(include_scores=True)) - own_score_map = self._own_score_map(request) + own_review_map = self._own_review_map(request) summary_map = self._summary_map(submissions) submissions.sort( key=lambda submission: ( @@ -168,7 +186,7 @@ def admin(self, request): serializer = FeatureCandidateAdminSerializer( submissions, many=True, - context={'own_score_map': own_score_map, 'summary_map': summary_map}, + context={'own_review_map': own_review_map, 'summary_map': summary_map}, ) return Response({'results': serializer.data}) diff --git a/frontend/src/lib/api.js b/frontend/src/lib/api.js index e0b60ce7..075065a4 100644 --- a/frontend/src/lib/api.js +++ b/frontend/src/lib/api.js @@ -324,7 +324,7 @@ export const stewardAPI = { // Feature candidate scoring getFeatureReviewAccess: () => api.get('/stewards/feature-reviews/access/'), getFeatureReviewCandidates: () => api.get('/stewards/feature-reviews/'), - scoreFeatureReviewCandidate: (id, score) => api.post(`/stewards/feature-reviews/${id}/score/`, { score }), + scoreFeatureReviewCandidate: (id, score, reason = '') => api.post(`/stewards/feature-reviews/${id}/score/`, { score, reason }), getFeatureReviewAdmin: () => api.get('/stewards/feature-reviews/admin/') }; diff --git a/frontend/src/routes/StewardFeatureReviews.svelte b/frontend/src/routes/StewardFeatureReviews.svelte index 4a419bc5..8762a585 100644 --- a/frontend/src/routes/StewardFeatureReviews.svelte +++ b/frontend/src/routes/StewardFeatureReviews.svelte @@ -56,6 +56,7 @@ let activeTab = $state('review'); let reviewFilterTab = $state('not_scored'); let saving = $state(new Set()); + let scoreDrafts = $state({}); let expandedTitles = $state(new Set()); let expandedDescriptions = $state(new Set()); let expandedAdminRows = $state(new Set()); @@ -116,6 +117,15 @@ const response = await stewardAPI.getFeatureReviewCandidates(); candidates = response.data?.results || []; progress = response.data?.progress || { scored: 0, total: candidates.length }; + scoreDrafts = Object.fromEntries( + candidates.map(candidate => [ + candidate.id, + { + score: candidate.own_score, + reason: candidate.own_score_reason || '', + }, + ]) + ); } async function loadAdminRows() { @@ -138,21 +148,64 @@ } } - async function scoreCandidate(candidate, score) { + function draftFor(candidate) { + return scoreDrafts[candidate.id] || { + score: candidate.own_score, + reason: candidate.own_score_reason || '', + }; + } + + function draftScore(candidate) { + return draftFor(candidate).score; + } + + function draftReason(candidate) { + return draftFor(candidate).reason || ''; + } + + function updateScoreDraft(candidate, patch) { + scoreDrafts = { + ...scoreDrafts, + [candidate.id]: { + ...draftFor(candidate), + ...patch, + }, + }; + } + + async function scoreCandidate(candidate) { + const draft = draftFor(candidate); + if (!Number.isInteger(draft.score)) { + showError('Select a score before saving.'); + return; + } + saving.add(candidate.id); saving = new Set(saving); try { - await stewardAPI.scoreFeatureReviewCandidate(candidate.id, score); + const response = await stewardAPI.scoreFeatureReviewCandidate( + candidate.id, + draft.score, + draft.reason || '' + ); + const savedReason = response.data?.reason ?? draft.reason ?? ''; const wasUnscored = candidate.own_score === null || candidate.own_score === undefined; candidates = candidates.map(item => - item.id === candidate.id ? { ...item, own_score: score } : item + item.id === candidate.id + ? { ...item, own_score: draft.score, own_score_reason: savedReason } + : item ); + updateScoreDraft(candidate, { score: draft.score, reason: savedReason }); if (wasUnscored) { progress = { ...progress, scored: progress.scored + 1 }; } showSuccess('Score saved'); } catch (err) { - showError(err.response?.data?.detail || err.response?.data?.score || 'Failed to save score.'); + const apiError = err.response?.data; + const scoreError = Array.isArray(apiError?.score) ? apiError.score[0] : apiError?.score; + const reasonError = Array.isArray(apiError?.reason) ? apiError.reason[0] : apiError?.reason; + const message = apiError?.detail || scoreError || reasonError || 'Failed to save score.'; + showError(typeof message === 'string' ? message : 'Failed to save score.'); } finally { saving.delete(candidate.id); saving = new Set(saving); @@ -308,12 +361,12 @@ } function scoreButtonClass(candidate, option) { - if (candidate.own_score === option.value) return option.selectedClass; + if (draftScore(candidate) === option.value) return option.selectedClass; return `${option.guideClass} hover:border-[#19A663] hover:bg-white hover:text-gray-950`; } function scoreNumberClass(candidate, option) { - if (candidate.own_score === option.value) return 'bg-white/20 text-current'; + if (draftScore(candidate) === option.value) return 'bg-white/20 text-current'; return option.numberClass; } @@ -564,12 +617,12 @@ {/if} -
+
{#each scoreOptions as option} {/each} + +
{/each}