Skip to content

Commit

Permalink
fix: add backend url validation for url fields (bloom-housing#3568)
Browse files Browse the repository at this point in the history
* fix: add backend url validation for url fields

* fix: validate only if externalUrl should be URL

* fix: use form values for application types on failed form submit

* fix: hide url field when no digital application

* fix: set proper default value to common digital application choice

* fix: add hasHttps decorator for second error variant
  • Loading branch information
KrissDrawing authored and ludtkemorgan committed Sep 5, 2023
1 parent 04b7695 commit 5a348e0
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import { Column, Entity, ManyToOne, OneToMany } from "typeorm"
import { Expose, Type } from "class-transformer"
import { IsBoolean, IsEnum, IsOptional, IsString, MaxLength, ValidateNested } from "class-validator"
import {
IsBoolean,
IsEnum,
IsOptional,
IsString,
IsUrl,
MaxLength,
ValidateIf,
ValidateNested,
} from "class-validator"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { AbstractEntity } from "../../shared/entities/abstract.entity"
import { ApiProperty } from "@nestjs/swagger"
import { Listing } from "../../listings/entities/listing.entity"
import { ApplicationMethodType } from "../types/application-method-type-enum"
import { PaperApplication } from "../../paper-applications/entities/paper-application.entity"
import { hasHttps } from "../../shared/decorators/hasHttps.decorator"

@Entity({ name: "application_methods" })
export class ApplicationMethod extends AbstractEntity {
Expand All @@ -29,6 +39,11 @@ export class ApplicationMethod extends AbstractEntity {
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsString({ groups: [ValidationsGroupsEnum.default] })
@MaxLength(4096, { groups: [ValidationsGroupsEnum.default] })
@ValidateIf((o) => o.type === ApplicationMethodType.ExternalLink, {
groups: [ValidationsGroupsEnum.default],
})
@hasHttps({ groups: [ValidationsGroupsEnum.default] })
@IsUrl({ require_protocol: true }, { groups: [ValidationsGroupsEnum.default] })
externalReference?: string | null

@Column({ type: "bool", nullable: true })
Expand Down
11 changes: 10 additions & 1 deletion backend/core/src/listings/entities/listing-event.entity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { Expose, Type } from "class-transformer"
import { IsDate, IsDefined, IsEnum, IsOptional, IsString, ValidateNested } from "class-validator"
import {
IsDate,
IsDefined,
IsEnum,
IsOptional,
IsString,
IsUrl,
ValidateNested,
} from "class-validator"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ListingEventType } from "../types/listing-event-type-enum"
import { ApiProperty } from "@nestjs/swagger"
Expand Down Expand Up @@ -42,6 +50,7 @@ export class ListingEvent extends AbstractEntity {
@Expose()
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsString({ groups: [ValidationsGroupsEnum.default] })
@IsUrl({ require_protocol: true }, { groups: [ValidationsGroupsEnum.default] })
url?: string | null

@Column({ type: "text", nullable: true })
Expand Down
2 changes: 2 additions & 0 deletions backend/core/src/listings/entities/listing.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
IsUUID,
MaxLength,
ValidateNested,
IsUrl,
} from "class-validator"
import { ApiProperty, ApiPropertyOptional } from "@nestjs/swagger"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
Expand Down Expand Up @@ -334,6 +335,7 @@ class Listing extends BaseEntity {
@Expose()
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsString({ groups: [ValidationsGroupsEnum.default] })
@IsUrl({ require_protocol: true }, { groups: [ValidationsGroupsEnum.default] })
buildingSelectionCriteria?: string | null

@ManyToOne(() => Asset, { eager: true, nullable: true, cascade: true })
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Expose } from "class-transformer"
import { IsString } from "class-validator"
import { IsString, IsUrl } from "class-validator"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ApiProperty } from "@nestjs/swagger"

Expand All @@ -12,5 +12,6 @@ export class MultiselectLink {
@Expose()
@IsString({ groups: [ValidationsGroupsEnum.default] })
@ApiProperty()
@IsUrl({ require_protocol: true }, { groups: [ValidationsGroupsEnum.default] })
url: string
}
31 changes: 31 additions & 0 deletions backend/core/src/shared/decorators/hasHttps.decorator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {
registerDecorator,
ValidationArguments,
ValidationOptions,
ValidatorConstraint,
ValidatorConstraintInterface,
} from "class-validator"

export function hasHttps(validationOptions?: ValidationOptions) {
return function (object: unknown, propertyName: string) {
registerDecorator({
target: object.constructor,
propertyName,
options: validationOptions,
constraints: [],
validator: hasHttpsConstraint,
})
}
}

@ValidatorConstraint({ name: "hasHttps" })
export class hasHttpsConstraint implements ValidatorConstraintInterface {
validate(url: string) {
const httpsRegex = /^https?:\/\//i
return httpsRegex.test(url)
}

defaultMessage(args: ValidationArguments) {
return `${args.property} must have https://`
}
}
2 changes: 1 addition & 1 deletion backend/core/test/listings/listings.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ describe("Listings", () => {
type: ListingEventType.openHouse,
startTime: new Date(),
endTime: new Date(),
url: "testurl",
url: "https://www.testurl.com",
note: "testnote",
label: "testlabel",
file: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ export const getDetailBoolean = (listingBool: boolean) => {
}

export const getReadableErrorMessage = (errorMessage: string | undefined) => {
const errorDetails = errorMessage.substr(errorMessage.indexOf(" ") + 1)
const errorDetails = errorMessage.substring(errorMessage.indexOf(" ") + 1)
let readableMessage = null
switch (errorDetails) {
case "must have https://":
readableMessage = t("errors.urlHttpsError")
break
case "must be a URL address":
readableMessage = t("errors.urlError")
break
case "must be an email":
readableMessage = t("errors.emailAddressError")
break
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ interface Methods {

const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { register, setValue, watch, errors } = useFormContext()
const { register, setValue, watch, errors, getValues } = useFormContext()
// watch fields
const digitalApplicationChoice = watch("digitalApplicationChoice")
const commonDigitalApplicationChoice = watch("commonDigitalApplicationChoice")
Expand Down Expand Up @@ -150,7 +150,12 @@ const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
paper: null,
referral: null,
}
listing?.applicationMethods?.forEach((method) => {
const applicationMethods =
getValues()?.applicationMethods?.length > 0
? getValues().applicationMethods
: listing?.applicationMethods

applicationMethods?.forEach((method) => {
switch (method.type) {
case ApplicationMethodType.Internal:
case ApplicationMethodType.ExternalLink:
Expand Down Expand Up @@ -256,7 +261,8 @@ const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
{
...yesNoRadioOptions[0],
id: "commonDigitalApplicationChoiceYes",
defaultChecked: listing?.commonDigitalApplication === true ?? null,
defaultChecked:
methods?.digital?.type === ApplicationMethodType.Internal ?? null,
inputProps: {
onChange: () => {
setMethods({
Expand All @@ -272,7 +278,8 @@ const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
{
...yesNoRadioOptions[1],
id: "commonDigitalApplicationChoiceNo",
defaultChecked: listing?.commonDigitalApplication === false ?? null,
defaultChecked:
methods?.digital?.type === ApplicationMethodType.ExternalLink ?? null,
inputProps: {
onChange: () => {
setMethods({
Expand All @@ -290,7 +297,9 @@ const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
</GridCell>
)}
</GridSection>
{((commonDigitalApplicationChoice && commonDigitalApplicationChoice === YesNoAnswer.No) ||
{((commonDigitalApplicationChoice &&
commonDigitalApplicationChoice === YesNoAnswer.No &&
digitalApplicationChoice === YesNoAnswer.Yes) ||
(digitalApplicationChoice === YesNoAnswer.Yes &&
!commonDigitalApplicationChoice &&
listing?.commonDigitalApplication === false)) && (
Expand All @@ -315,6 +324,8 @@ const ApplicationTypes = ({ listing }: { listing: FormListing }) => {
})
},
}}
error={fieldHasError(errors?.applicationMethods?.[0]?.externalReference)}
errorMessage={fieldMessage(errors?.applicationMethods?.[0]?.externalReference)}
/>
</GridCell>
</GridSection>
Expand Down

0 comments on commit 5a348e0

Please sign in to comment.