Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 143 additions & 0 deletions CONTRIBUTION_ANALYSIS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# Montage Project Contribution Analysis

## Overview

**Montage** is a Vue.js + Python/Clastic photo evaluation tool for Wiki Loves competitions. It manages campaign coordination and juror voting workflows with different round types (rating, ranking, yes/no).

### Current Work Status

**Recent Contributions:**
- **PR #404**: Frontend validation display + backend ranking round fix (100-submit cap)
- **PR #410**: Round creation preemptive checks validation (addressing issue #103)

---

## Key Stability Issues & Areas for Contribution

### 1. **Input Validation Gaps**

**Frontend Issues:**
- [[frontend/src/components/](frontend/src/components/)] Components lack consistent validation feedback
- Round creation form doesn't validate conflicting parameters before submission
- Campaign date inputs need cross-field validation (end_date > start_date)
- Voter input fields lack real-time validation for rating/ranking rounds
- File upload inputs (CSV imports) have minimal validation before sending to backend

**Backend Issues:**
- [[montage/admin_endpoints.py](montage/admin_endpoints.py)] Most POST endpoints lack input parameter validation
- `create_round()` function doesn't validate threshold_map structure before persisting
- No validation of vote values against VALID_RATINGS/VALID_YESNO before storage
- Missing null/empty checks in DAO methods (e.g., `add_juror()`)

**Impact**: Prevents 400/500 errors and null value bugs. Improves UX with early feedback.

---

### 2. **Error Handling Deficiencies**

**Frontend:**
- [[frontend/src/services/api.js](frontend/src/services/api.js)] Generic error toast instead of detailed backend messages
- Vote submission errors during fast voting don't show which entries failed (issue #206)
- Network failures during CSV import/round creation lack user feedback
- Missing retry logic for transient failures

**Backend:**
- [[montage/mw/__init__.py](montage/mw/__init__.py#L50-L90)] TODO comment: need to autoswitch HTTP status codes based on response_dict['status']
- Generic 500 errors for missing resources (issue #400 context) instead of proper 404/403
- `check_entry_existence()` calls [[montage/rdb.py](montage/rdb.py)] but no centralized error handling
- Vote submission with >100 entries returns unclear error

**Impact**: Better user experience, easier debugging, proper HTTP semantics.

---

### 3. **Test Coverage Gaps**

**Missing Test Paths:**
- No tests for round creation edge cases (conflicting threshold values, invalid vote methods)
- Vote submission with fast clicking (race condition in issue #206)
- CSV import with invalid/partial data ([[montage/loaders.py](montage/loaders.py#L77-L200)]) - only has warnings, no tests
- API contract violations (e.g., ranking round >100 entries)
- Concurrent vote updates from multiple jurors
- Permission checks (coordinator access to campaigns/rounds)

**Existing Tests:**
- [[montage/tests/test_web_basic.py](montage/tests/test_web_basic.py)] - Basic routing only
- [[montage/tests/test_loaders.py](montage/tests/test_loaders.py)] - Loader validation only
- [[frontend/cypress/e2e/](frontend/cypress/e2e/)] - Campaign/login E2E tests exist but incomplete

**Impact**: Catches regressions, documents expected behavior, prevents ticket repeats.

---

### 4. **API Contract Definition Issues**

**Unclear Contracts:**
- `POST /admin/add_series` response data marked TODO - should match `series_info` schema
- `POST /admin/add_organizer` response TODO - inconsistent with other user endpoints
- Vote statistics endpoint (`/juror/round/<id>/votes-stats`) lacks documented response shape
- [[montage/docs/api.md](montage/docs/api.md#L20-L50)] Multiple TODOs for standardizing response shapes
- No OpenAPI/JSON schema for validation

**Database Schema Issues:**
- [[montage/check_rdb.py](montage/check_rdb.py#L18-L60)] Relationships not verified at startup
- Missing foreign key constraints in some tables
- No documented cascade delete behavior

**Impact**: Prevents frontend/backend mismatches, enables API testing, improves developer experience.

---

## Specific Issues from GSoC Requirements

| Issue | Status | Area | Priority |
|-------|--------|------|----------|
| #400 | PR #404 in review | Round 100-vote validation | High |
| #103 | PR #410 in review | Round creation validation | High |
| #206 | Open | Fast voting race condition | Medium |
| #325 | Open | Vote statistics display | Medium |
| #322 | Open | Edit vote loading | Medium |
| #155 | Open | Author/upload date from file v1 | Low |

---

## Recommended Contribution Path

### Phase 1: Foundation (Low-hanging fruit)
1. **Add input validation** to frontend form components (round creation, campaign dates)
2. **Improve error messages** - Replace generic toasts with backend error details
3. **Write integration tests** for CSV import edge cases

### Phase 2: Core Stability
4. **Add backend validation** for POST endpoint parameters
5. **Fix fast-voting race condition** (issue #206) with proper locking/atomicity
6. **Define API contracts** - Document response schemas for all endpoints

### Phase 3: Polish
7. **Add comprehensive test suite** for permission checks
8. **Improve voting statistics display** (issue #325) with error state handling
9. **Fix edit vote loading** (issue #322) with proper async state management

---

## Code Quality Quick Wins

**Backend TODOs to Address:**
- Line 58 in `mw/__init__.py`: Autoswitch HTTP status codes
- Line 366-368 in `rdb.py`: Verify count_map logic
- Multiple "TODO" markers in API docs for standardization

**Frontend TODOs:**
- Configurable backend URLs for different deploy environments
- Cookie-based "show directions" preference
- Threshold unit translation (e.g., ">0.5" → "at least 2 votes")

---

## Development Environment Notes

- **Backend**: Python 3, Clastic framework, SQLAlchemy ORM
- **Frontend**: Vue 3, Vite, Axios, Cypress E2E tests
- **Database**: MySQL via Toolforge
- **Key Dependencies**: SQLAlchemy, Werkzeug, PyMySQL
- **Testing**: Tox, pytest (backend), Cypress (frontend)
228 changes: 228 additions & 0 deletions IMPLEMENTATION_SUMMARY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
# Implementation Summary: Frontend Form Validation

## Overview
Successfully implemented comprehensive input validation for campaign and round creation/editing forms across the Montage application frontend. This addresses the "Add frontend form validation for round/campaign creation" task by preventing invalid data submission and providing clear, real-time feedback to users.

## Changes Made

### 1. Configuration & Localization (frontend/src/i18n/en.json)
Added 10 new validation error messages:
```json
{
"montage-error-close-after-open": "Close date and time must be after open date and time",
"montage-error-deadline-required": "Voting deadline is required",
"montage-error-round-name-required": "Round name is required",
"montage-error-quorum-positive": "Quorum must be a positive number",
"montage-error-jurors-required": "At least one juror is required for this round",
"montage-error-threshold-required": "Threshold must be selected",
"montage-error-import-source-required": "Please select an import source",
"montage-error-category-required": "Category is required",
"montage-error-csv-url-required": "CSV URL is required",
"montage-error-file-list-required": "File list is required"
}
```

### 2. Campaign Creation (frontend/src/components/Campaign/NewCampaign.vue)
**Before:**
- Basic individual field validation using Zod
- Missing cross-field validation

**After:**
- ✅ Zod schema with `.refine()` for cross-field validation
- ✅ Ensures close date/time is after open date/time
- ✅ Prevents form submission with invalid date ranges
- ✅ Displays inline error messages in form fields

### 3. Campaign Editing (frontend/src/components/Campaign/ViewCampaign.vue)
**Before:**
- Basic schema validation only
- No cross-field date validation

**After:**
- ✅ Added same date range validation as campaign creation
- ✅ Applied to edit mode with same error handling
- ✅ Consistent validation across create/edit flows

### 4. Round Creation (frontend/src/components/Round/RoundNew.vue) - **Most Comprehensive**
**Before:**
- Minimal validation: only checked deadline, name, and jurors if quorum > 0
- No field-level error tracking
- No UI feedback for validation errors
- Missing validations for:
- Round name requirements
- Quorum value validation (positive number)
- Import source data (category, CSV, file list)
- Threshold selection for subsequent rounds

**After:**
- ✅ Comprehensive validation function `validateRound()`
- ✅ Error tracking ref with 7 error fields
- ✅ Field-level error display with Codex styling
- ✅ All required validations implemented:
- Round name (non-empty)
- Deadline date (required)
- Vote method (selected)
- Quorum (positive integer)
- Jurors (at least one)
- Import source data (first round only)
- Threshold selection (subsequent rounds only)
- ✅ Form fields updated with `:status` and `:messages` bindings
- ✅ Type-safe quorum validation (parseInt with NaN check)

## Validation Logic Details

### Campaign/Campaign Edit Validation
```javascript
// Individual field validations (Zod)
- name: required string
- url: required, valid URL format
- openDate: valid date format
- openTime: valid time format (HH:mm)
- closeDate: valid date format
- closeTime: valid time format (HH:mm)
- coordinators: array with at least 1 element

// Cross-field validation
- closeDateTime > openDateTime
```

### Round Validation
```javascript
// All rounds must have:
- name (non-empty string)
- deadline_date (required)
- vote_method (selected)
- quorum (positive integer > 0)
- jurors (at least 1 juror)

// First round (roundIndex === 0) additionally requires:
- Import source selection + data:
- Category: category name selected
- CSV: URL provided and non-empty
- File list: list provided and non-empty

// Subsequent rounds (roundIndex > 0) additionally require:
- threshold: must be selected from available thresholds
```

## UI Enhancements

### Error Display
- Errors appear inline below form fields
- Codex `cdx-field` components display error status visually
- Error messages are translatable from i18n
- Real-time clearing of previous errors before validation

### Form Fields with Error Support
**NewCampaign & ViewCampaign:**
- Campaign name
- Campaign URL
- Open date & time (pair validation)
- Close date & time (pair validation)
- Coordinators

**RoundNew:**
- Round name
- Deadline date
- Vote method
- Quorum
- Jurors list
- Threshold (conditional)

## Impact & Benefits

### Prevents Application Errors
- ✅ Stops submission of invalid date ranges (campaign close before open)
- ✅ Prevents null/empty values in required fields
- ✅ Validates quorum is a positive number
- ✅ Ensures import sources have data before processing

### Improves User Experience
- ✅ Clear, descriptive error messages
- ✅ Immediate feedback (error messages appear on validation)
- ✅ Field-level error indication (red border/styling)
- ✅ Prevents wasted server calls with invalid data
- ✅ Reduced frustration from generic backend errors

### Reduces Server Load
- ✅ Eliminates invalid requests before reaching backend
- ✅ Prevents 400/500 errors from malformed data
- ✅ Reduces database transaction attempts for invalid data

### Code Quality
- ✅ Centralized validation logic (validateRound function)
- ✅ Reusable error tracking pattern
- ✅ Type-safe number conversion
- ✅ Clean error state management

## Testing Checklist

### Manual Testing Scenarios

**Campaign Creation:**
- [ ] Empty campaign name → shows error
- [ ] Invalid URL format → shows error
- [ ] Close date before open date → shows error
- [ ] No coordinators selected → shows error
- [ ] All valid inputs → submits successfully

**Campaign Editing:**
- [ ] All campaign creation scenarios + date range validation
- [ ] Editing with invalid dates → shows error

**Round Creation - First Round:**
- [ ] Empty round name → shows error
- [ ] No deadline selected → shows error
- [ ] Quorum = 0 or negative → shows error
- [ ] No jurors selected → shows error
- [ ] Category selected, no category value → shows error
- [ ] CSV selected, no URL → shows error
- [ ] File list selected, no files → shows error
- [ ] All valid inputs → submits successfully

**Round Creation - Subsequent Rounds:**
- [ ] All first round validations (except import source)
- [ ] No threshold selected → shows error
- [ ] Threshold selected → submits successfully

## Files Summary

| File | Changes | Impact |
|------|---------|--------|
| en.json | +12 lines | Added 10 error message translations |
| NewCampaign.vue | +/-5 lines | Added cross-field date validation |
| ViewCampaign.vue | +/-5 lines | Added cross-field date validation |
| RoundNew.vue | +/-76 lines | Comprehensive validation implementation |
| **Total** | **+111/-18** | **93 net additions** |

## Next Steps (Optional Enhancements)

1. **Field-level debounced validation** - Validate fields as user types
2. **Success visual feedback** - Show checkmarks for valid fields
3. **Loading states** - Better UX during form submission
4. **Advanced validations**:
- CSV/Google Sheet import validation
- Minimum resolution settings validation
- Username availability check for jurors/coordinators
5. **Accessibility improvements**:
- ARIA labels for error messages
- Keyboard navigation enhancements
6. **Multi-error support** - Show multiple error messages per field if needed

## Verification

✅ All files successfully modified
✅ Schema validations correctly implemented
✅ Error tracking refs initialized
✅ Form fields updated with error status bindings
✅ Validation functions properly clear and set errors
✅ All new translation keys added
✅ Git diff shows expected changes (~111 insertions, 18 deletions)

## Notes

- The quorum field uses `input-type="number"` which should be coerced to number by Vue, but validation includes `parseInt()` for safety
- Validation prevents form submission by returning early if `validateRound()` returns false
- Error messages are translatable, allowing easy multi-language support
- The implementation follows Montage's existing patterns (Codex components, i18n, Zod validation)
- No breaking changes to existing functionality
Loading