Add max length limitation to open questions and raise error for when travel fields are empty and travel is selected

[estyxx]

• Enforce travel detail fields for grant applications in backend
• Remove django-debug-toolbar from test config to fix local test failure
• Add character limitations for all open question fields

What

ToDo

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Updated (UTC)
pycon	Ready	Preview	Nov 30, 2025 3:19pm

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.49%. Comparing base (6686fae) to head (749091e).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4501      +/-   ##
==========================================
- Coverage   94.53%   94.49%   -0.04%     
==========================================
  Files         352      352              
  Lines       10131    10133       +2     
  Branches      733      734       +1     
==========================================
- Hits         9577     9575       -2     
- Misses        461      463       +2     
- Partials       93       95       +2     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[marcoacierno]

@claude do a code review

[claude]

Claude finished @marcoacierno's task —— View job

---

Code Review

I've completed a comprehensive review of PR #4501. Here's my detailed feedback:

Summary

This PR adds character limitations to grant application text fields and enforces that travel detail fields (departure country, city, nationality) are required when needsFundsForTravel is true. The changes are well-implemented overall with good test coverage.

---

✅ Strengths

1. Backend validation is solid: The validation logic in backend/api/grants/mutations.py:66-105 correctly enforces both max length and required field constraints
2. Good test coverage: The new tests adequately cover both max length validation and required travel fields scenarios
3. Frontend enforcement: Max length attributes are properly added to form fields
4. Test configuration fix: Removing django-debug-toolbar from test config is the right solution

---

🔍 Issues Found

1. Missing maxLength enforcement on fullName and name fields (frontend/src/components/grant-form/index.tsx)

Severity: Medium

The backend enforces 300 character limits on fullName and name fields (lines 68-69 in mutations.py), but the frontend Input components don't have maxLength attributes:

• Line 355-360: fullName input missing maxLength={300}
• Line 370-375: name input missing maxLength={300}

Recommendation: Add maxLength={300} to both Input components for consistency with backend validation and better UX.

2. Inconsistent required attribute on travel fields (frontend/src/components/grant-form/index.tsx)

Severity: Low

Lines 589-634: When needsFundsForTravel === "true", the travel fields show required={false} in InputWrapper but required={true} on the actual Input/Select components. This is confusing.

Current code:

<InputWrapper required={false}>
  <Input {...text("departureCity")} required={true} ... />
</InputWrapper>

Recommendation: Change InputWrapper required={false} to required={true} on lines 590 and 619 to match the actual field requirement.

3. Test coverage gap (Codecov report)

Severity: Low

Codecov reports 4 lines missing coverage (75% patch coverage). While the existing tests are good, consider adding edge case tests:

• What happens when needs_funds_for_travel is False but travel fields are provided?
• Test with exactly max length characters (boundary testing)
• Test update_grant mutation with the new validations

---

💡 Additional Observations

Backend (mutations.py)

Lines 94-97: The conditional addition of required fields is clean and correct:

if self.needs_funds_for_travel:
    non_empty_fields.extend(
        ["departure_country", "departure_city", "nationality"]
    )

Lines 78-85: The max length validation loop is efficient. One minor suggestion - consider extracting the error message format to a constant for easier i18n in the future.

Test file (test_send_grant.py)

Lines 230-280: test_cannot_send_grant_outside_allowed_values is comprehensive and tests all max length limits properly.

Lines 307-334: test_cannot_send_grant_with_empty_values_if_needs_funds_for_travel correctly validates the conditional required fields.

Good practice: The tests properly use the factory pattern and check both success and error scenarios.

Settings (test.py)

Lines 6-11: The django-debug-toolbar removal is correct and addresses the test failure mentioned in the PR description. The conditional removal prevents errors if it's already been removed.

---

📋 Summary of Recommendations

1. Must fix: Add maxLength={300} to fullName and name Input components (frontend)
2. Should fix: Update InputWrapper required attribute to true for travel fields when they're conditionally required
3. Nice to have: Add edge case tests to improve coverage to 100%
4. Consider: Extract error message format strings for potential i18n

---

Final Verdict

The PR is in good shape and ready for merge after addressing the missing maxLength attributes on the frontend. The other issues are minor and could be addressed in a follow-up PR if preferred.

Great work on the implementation! The backend validation is robust and the test coverage demonstrates the functionality works as expected.

---