Skip to content

fix: return all validation errors from /api/recommend - #250

Closed
Tranquil666 wants to merge 2 commits into
komalharshita:mainfrom
Tranquil666:fix/return-all-validation-errors
Closed

fix: return all validation errors from /api/recommend#250
Tranquil666 wants to merge 2 commits into
komalharshita:mainfrom
Tranquil666:fix/return-all-validation-errors

Conversation

@Tranquil666

@Tranquil666 Tranquil666 commented May 17, 2026

Copy link
Copy Markdown

Summary [required]

The /api/recommend endpoint collected all validation errors via validate_recommendation_inputs() but only ever returned errors[0] to the client. This forced users to fix one field, resubmit, discover the next error, and repeat — a frustrating loop when multiple fields are invalid at once. This PR returns the full errors list so every problem is surfaced in a single response.

A second minor fix in the same file moves import os from inside the download_code() function body to the module-level imports, which is required by PEP 8 (E402) and the project's code style rules.

Related Issue [required]

Closes #

Type of Change [required]

  • Bug fix — resolves a broken behaviour
  • Refactor — restructures code without changing behaviour

What Was Changed [required]

File Change made
routes/main_routes.py Return {"errors": errors} (list) instead of {"error": errors[0]} (string); move import os to module-level
CHANGELOG.md Added entry under ### Fixed as required by CONTRIBUTING.md for user-facing changes

How to Test This PR [required]

  1. Clone this branch: git checkout fix/return-all-validation-errors
  2. Install dependencies: pip install -r requirements.txt
  3. Run the app: python app.py
  4. Open http://127.0.0.1:5000 and submit the form with all four fields empty
  5. Confirm the response contains an "errors" key with all four error messages, not just one
  6. Submit with only one invalid field — confirm a single-item list is returned
  7. Submit a fully valid payload — confirm {"projects": [...]} response is unchanged
  8. Navigate to any project detail page and click Download — confirm the file downloads correctly
  9. Run the tests: python tests/test_basic.py

Expected test output:

29 passed, 0 failed out of 29 tests

Test Results [required]

  PASS  test_projects_json_loads
  PASS  test_each_project_has_required_fields
  PASS  test_find_project_by_id_found
  PASS  test_find_project_by_id_missing
  PASS  test_parse_skills_basic
  PASS  test_parse_skills_empty_string
  PASS  test_parse_skills_single_entry
  PASS  test_score_single_project_full_match
  PASS  test_score_single_project_no_match
  PASS  test_get_recommendations_returns_results
  PASS  test_get_recommendations_max_three
  PASS  test_get_recommendations_no_match_returns_empty
  PASS  test_get_recommendations_result_format
  PASS  test_validate_all_valid
  PASS  test_validate_missing_skills
  PASS  test_validate_missing_level
  PASS  test_validate_missing_interest
  PASS  test_validate_missing_time
  PASS  test_validate_all_missing
  PASS  test_home_route
  PASS  test_recommend_api_valid
  PASS  test_recommend_api_missing_field
  PASS  test_recommend_api_empty_body
  PASS  test_project_detail_found
  PASS  test_project_detail_not_found
  PASS  test_internal_server_error_page
  PASS  test_view_code_found
  PASS  test_download_code_found
  PASS  test_scoring_weights_has_all_keys

29 passed, 0 failed out of 29 tests

Self-Review Checklist [required]

  • I have read CONTRIBUTING.md and followed all guidelines
  • My branch name follows the convention: fix/return-all-validation-errors
  • I have run python tests/test_basic.py and all 29 tests pass
  • I have run flake8 routes/main_routes.py locally and there are no errors
  • I have not introduced any print() or console.log() debug statements
  • Every new function I wrote has a docstring
  • I have not modified files outside the scope of the linked issue

Notes for Reviewer

The response key changes from "error" (string) to "errors" (list). If any frontend JavaScript currently reads response.error, it will need to switch to response.errors or response.errors[0]. Worth checking static/ JS before merging.

Pre-existing flake8 violations exist in starter_code/ and utils/ but are unrelated to this PR — routes/main_routes.py is clean.

Previously only errors[0] was returned, forcing users to fix and
resubmit one field at a time. The response key is now "errors" (list).
Also moves `import os` to module level in download_code().
@vercel

vercel Bot commented May 17, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added gssoc-2026 type:bug Something isn't working labels May 17, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting your first pull request to DevPath.

Before review:

  • Complete the PR template fully
  • Ensure all tests pass
  • Link your PR to an issue
  • Keep changes scoped to the issue

A maintainer will review your contribution soon.

@komalharshita komalharshita left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API validation improvement itself looks good conceptually, and returning all validation errors is a better user experience overall.

However, the CI checks are currently failing, so this PR cannot be safely merged yet.

Since the response structure changed from:

{ "error": "..." }

to:

{ "errors": [...] }

the failing checks may indicate broken tests or frontend compatibility issues related to the updated API response format.

Please:

  • resolve all failing CI checks
  • verify frontend/client compatibility with the new response structure
  • update/add tests if necessary

Once CI passes and compatibility is confirmed, this can be re-reviewed.

test_recommend_api_missing_field was asserting the old "error" string
key — updated to assert "errors" list key and verify it is non-empty.

static/script.js was reading data.error (string) from /api/recommend —
updated to read data.errors (list) and join all messages for display so
the user sees every validation problem at once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:invalid This doesn't seem right gssoc-2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants