Skip to content

Improve API Request Validations - #194

Open
betsyecastro wants to merge 10 commits into
developfrom
improve-api-request-validations
Open

Improve API Request Validations#194
betsyecastro wants to merge 10 commits into
developfrom
improve-api-request-validations

Conversation

@betsyecastro

Copy link
Copy Markdown
Contributor

Implements the following API request validation rules:

  • Person (slug): Must contain only alphanumeric characters and periods.
  • Search fields: Must be at least three characters long.
  • School: Must match one of the whitelisted school short names.
  • With data: Allowed only when the person parameter is present.

@betsyecastro
betsyecastro requested a review from wunc May 13, 2025 15:49
@betsyecastro betsyecastro self-assigned this May 13, 2025
@betsyecastro betsyecastro added 🐛 bug Something isn't working ✨ enhancement New feature or request ⬆️ priority:high High priority issue labels May 13, 2025
@betsyecastro betsyecastro changed the title Improve API request validations Improve API Request Validations May 20, 2025
@betsyecastro
betsyecastro marked this pull request as draft May 20, 2025 16:55
@betsyecastro
betsyecastro marked this pull request as ready for review June 5, 2025 18:51
@shukla-m
shukla-m self-requested a review June 9, 2025 20:23
@shukla-m

shukla-m commented Jun 9, 2025

Copy link
Copy Markdown

@betsyecastro I added myself as a reviewer on this PR, as we discussed last week, and pulled in the code to test locally. I tried the following URL which passes validation for the person and with_data fields. The person query parameter is invalid - yes I was trying to break it! 😃

http://profiles.test/api/v1?with_data=1&data_type=information&public=1&person=herve;abdi

This might not be an issue but wanted to mention, in case ProfileApiRequest.php@validateWithData() method needed to check for valid person in addition to the existing check for non-empty person. Let me know if you would like to look at it together or discuss. Thanks.

@shukla-m shukla-m 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.

Looks good! I tested the updates locally and it seemed to work as expected. See comment regarding person query parameter with invalid value, in case it causes any issues.

betsyecastro and others added 4 commits June 17, 2025 10:44
- use more thorough search pattern regex (for example, some profile slugs may have dashes and quotes)
- add `search_section` and `accepting_undergrad` as valid params
- simplify the `with_data` validation by using the built-in `Rule::prohibitedIf()`
- also allow `with_data` if `data_type` is filled
- for extra param checking, use the `after()` method instead of `passedValidation()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working ✨ enhancement New feature or request ⬆️ priority:high High priority issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants