PhilanthropyDataCommons / service

A project for collecting and serving public information associated with grant applications
GNU Affero General Public License v3.0
8 stars 2 forks source link

Validate URLs and Emails using ajv formats #875

Closed hminsky2002 closed 6 months ago

hminsky2002 commented 7 months ago

Related to issue #760

The current mode of email and URL validation in proposalFieldValues(src/fieldValidation.ts) is done using regexes which are a temporary solution. Validation for URLs and Emails should be done using ajv-formats https://github.com/ajv-validator/ajv-formats.

hminsky2002 commented 7 months ago

@slifty It seems like the ajv-formats package has deprecated it's URL format in favor of it's URI format

Screen Shot 2024-04-08 at 7 22 49 PM

I wonder then if it might make more sense to use their broader URI format to validate and perhaps refactor the code from PR #867?

hminsky2002 commented 7 months ago

@slifty I've pushed to https://github.com/PhilanthropyDataCommons/service/tree/875-validate-email-url-with-ajv with just an email related refactor! Something else I've thought of is that since ajv-formats covers string/boolean/number, would it be a good idea to increase the scope of this issue to refactor those with ajv as well?

slifty commented 7 months ago

Good thought -- rather than increase scope of this issue lets make a new issue for checking certain primitive values using ajv! I think there are a few cases where we might be manually doing a check in that regard even outside of field-level validation.

slifty commented 7 months ago

Hmmmmmmmmmmmm

OK so we definitely shouldn't use the deprecated URL format (it'll be removed)

That said, URI isn't quite right because URI can either be a URL or a URN, and we don't want a URN.

See this: https://github.com/orgs/json-schema-org/discussions/205

If the URI validator of ajv-format will allow to specifically validate a url rather than a urn then great; otherwise I think we need to find another solution. This seems to be the issue that sparked url deprecation: https://github.com/ajv-validator/ajv/issues/898

hminsky2002 commented 7 months ago

@slifty Noted! I think on second look that the uri format in ajv is implicitly a full URI with protocol, whereas the uri-reference allows for URNs. On testing, with schema

const uriSchema = {
    type: 'string',
    format: 'uri',
}
console.log(ajv.validate(uriSchema, 'https://www.crud.com')) === true;

whereas

ajv.validate(uriSchema, 'www.crud.com') === false

To that end it seems like it would be ok to just use uri? I think it might just boil down to a syntactical difference.

A simple workaround might be to just rename

const uriSchema = {
    type: 'string',
    format: 'uri',
}

to

const urlSchema = {
    type: 'string',
    format: 'uri',
}
slifty commented 7 months ago

@hminsky2002 I wouldn't munge the naming so long as the outcome is what we want

hminsky2002 commented 7 months ago

@hminsky2002 I wouldn't munge the naming so long as the outcome is what we want

That seems wise, so keep it as

const uriSchema = {
    type: 'string',
    format: 'uri',
} 

but otherwise reference as URL(in the database and the Basefield Type)?