Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACM-14557-Implement inferred ranges for cluster list advanced search #3913

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 43 additions & 36 deletions frontend/src/lib/search-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,68 +2,75 @@
import { SearchOperator } from '../ui-components/AcmSearchInput'
import { handleStandardComparison, handleSemverOperatorComparison } from './search-utils'

const stringData = {
stringOne: 'Adam',
stringTwo: 'Becky',
stringThree: 'Charlie',
stringFour: 'David',
stringFive: 'Eve',
}
const versionData = {
versionOne: '1.0.0',
versionTwo: '1.5.0',
versionThree: '2.0.0',
versionFour: '2.5.0',
versionIncomplete: '1.',
}

describe('search-utils basic string operator', () => {
const { stringOne, stringTwo, stringThree, stringFour, stringFive } = stringData
it('can determine equals', () => {
expect(handleStandardComparison(stringOne, stringOne, SearchOperator.Equals)).toBeTruthy()
expect(handleStandardComparison('Adam', 'Adam', SearchOperator.Equals)).toBeTruthy()
})
it('can determine greater', () => {
expect(handleStandardComparison(stringOne, stringTwo, SearchOperator.GreaterThan)).toBeTruthy()
expect(handleStandardComparison('Adam', 'Becky', SearchOperator.GreaterThan)).toBeTruthy()
})
it('can determine less', () => {
expect(handleStandardComparison(stringFour, stringThree, SearchOperator.LessThan)).toBeTruthy()
expect(handleStandardComparison('David', 'Charlie', SearchOperator.LessThan)).toBeTruthy()
})
it('can determine greater than or equal to', () => {
expect(handleStandardComparison(stringOne, stringOne, SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison(stringOne, stringTwo, SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison('Adam', 'Adam', SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison('Adam', 'Becky', SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
})
it('can determine less than or equal to', () => {
expect(handleStandardComparison(stringOne, stringOne, SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison(stringFive, stringOne, SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison('Adam', 'Adam', SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleStandardComparison('Eve', 'Adam', SearchOperator.LessThanOrEqualTo)).toBeTruthy()
})
it('can determine non-equals', () => {
expect(handleStandardComparison(stringOne, stringFive, SearchOperator.Equals)).toBeFalsy()
expect(handleStandardComparison('Adam', 'Eve', SearchOperator.Equals)).toBeFalsy()
})
})

describe('search-utils sermver operator', () => {
const { versionOne, versionTwo, versionThree, versionFour, versionIncomplete } = versionData
it('can determine greater semver', () => {
expect(handleSemverOperatorComparison(versionFour, versionThree, SearchOperator.GreaterThan)).toBeTruthy()
it('can determine greater than semver', () => {
expect(handleSemverOperatorComparison('2.5.0', '2.0.0', SearchOperator.GreaterThan)).toBeTruthy()
})
it('can determine less semver', () => {
expect(handleSemverOperatorComparison(versionOne, versionTwo, SearchOperator.LessThan)).toBeTruthy()
it('can determine less than semver', () => {
expect(handleSemverOperatorComparison('1.0.0', '1.5.0', SearchOperator.LessThan)).toBeTruthy()
})
it('can determine equals semver', () => {
expect(handleSemverOperatorComparison(versionOne, versionOne, SearchOperator.Equals)).toBeTruthy()
expect(handleSemverOperatorComparison('1.0.0', '1.0.0', SearchOperator.Equals)).toBeTruthy()
})
it('can determine greater than or equal to semver', () => {
expect(handleSemverOperatorComparison(versionThree, versionThree, SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison(versionFour, versionThree, SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison('2.0.0', '2.0.0', SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison('2.5.0', '2.0.0', SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
})
it('can determine less than or equal to semver', () => {
expect(handleSemverOperatorComparison(versionTwo, versionTwo, SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison(versionOne, versionTwo, SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison('1.5.0', '1.5.0', SearchOperator.LessThanOrEqualTo)).toBeTruthy()
expect(handleSemverOperatorComparison('1.0.0', '1.5.0', SearchOperator.LessThanOrEqualTo)).toBeTruthy()
})
it('can determine non-equal semver', () => {
expect(handleSemverOperatorComparison(versionOne, versionTwo, SearchOperator.NotEquals)).toBeTruthy()
expect(handleSemverOperatorComparison('1.0.0', '1.5.0', SearchOperator.NotEquals)).toBeTruthy()
})
it('can coerce incomplete semver strings', () => {
expect(handleSemverOperatorComparison(versionIncomplete, versionOne, SearchOperator.Equals)).toBeTruthy()
expect(handleSemverOperatorComparison('1.', '1.0.0', SearchOperator.Equals)).toBeTruthy()
})
it('can infer on incomplete semver equals', () => {
expect(handleSemverOperatorComparison('1.0.0', '1.', SearchOperator.Equals)).toBeTruthy()
})
it('can infer on incomplete semver greater than', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails with my proposed PR, which makes sense if we consider >1 to mean 2 or higher. (1.5.0 is the first arg here)

By the way, using constants for the test values here just makes it hard to understand and debug the tests. I would just repeat the numbers in the tests.

expect(handleSemverOperatorComparison('2.5.0', '1.', SearchOperator.GreaterThan)).toBeTruthy()
})
it('can infer on incomplete semver greater than or equal to', () => {
expect(handleSemverOperatorComparison('1.5.0', '1.', SearchOperator.GreaterThanOrEqualTo)).toBeTruthy()
})
it('can infer on incomplete semver less than', () => {
expect(handleSemverOperatorComparison('1.0.0', '1.5', SearchOperator.LessThan)).toBeTruthy()
})
it('can infer on incomplete semver less than or equal to', () => {
expect(handleSemverOperatorComparison('1.0.0', '2', SearchOperator.LessThanOrEqualTo)).toBeTruthy()
})
it('can infer on incomplete semver not equals', () => {
expect(handleSemverOperatorComparison('1.0.0', '2.5', SearchOperator.NotEquals)).toBeTruthy()
})
it('can infer with incomplete semver form x.', () => {
expect(handleSemverOperatorComparison('1.', '2.5.', SearchOperator.LessThan)).toBeTruthy()
})
it('can infer with incomplete semver form x.y.', () => {
expect(handleSemverOperatorComparison('2.5.0', '2.5.', SearchOperator.Equals)).toBeTruthy()
})
})
42 changes: 24 additions & 18 deletions frontend/src/lib/search-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,38 @@ export const handleStandardComparison = (valueOne: string, valueTwo: string, ope
}
}

// for a given displayVersion string there is a distribution value a ' ' and a semver value, here we divide them and take the semver
export const handleSemverOperatorComparison = (versionOne: string, versionTwo: string, operator: SearchOperator) => {
// Semver coerces the version to a valid semver version if possible, otherwise it returns the original value
const coercedVersionOne = semver.valid(semver.coerce(versionOne)) ?? versionOne
const coercedVersionTwo = semver.valid(semver.coerce(versionTwo)) ?? versionTwo
export const handleSemverOperatorComparison = (versionToCompare: string, input: string, operator: SearchOperator) => {
const coercedInputVersion = semver.coerce(input)
const coercedVersionToCompare = semver.coerce(versionToCompare)

const validInputSemvers = !!semver.valid(coercedVersionOne) && !!semver.valid(coercedVersionTwo)
if (!validInputSemvers) {
if (operator === SearchOperator.NotEquals) {
return true
}
return false
if (!coercedInputVersion || !coercedVersionToCompare) {
// No valid semver in either case, so assume these are not equal
return operator === SearchOperator.NotEquals
}

// if user enters partial semver, coerce will assume the missing components are 0
// coerce is still useful in case the user enters more text than just a semver
let inputVersion = `${coercedInputVersion.major}.${coercedInputVersion.minor}.${coercedInputVersion.patch}`
if (!input.includes(inputVersion)) {
inputVersion = `${coercedInputVersion.major}.${coercedInputVersion.minor}`
}
if (!input.includes(inputVersion)) {
inputVersion = `${coercedInputVersion.major}`
}

switch (operator) {
case SearchOperator.Equals:
return semver.eq(coercedVersionOne, coercedVersionTwo)
return semver.satisfies(coercedVersionToCompare, inputVersion)
case SearchOperator.GreaterThan:
return semver.gt(coercedVersionOne, coercedVersionTwo)
case SearchOperator.LessThan:
return semver.lt(coercedVersionOne, coercedVersionTwo)
return semver.satisfies(coercedVersionToCompare, `>${inputVersion}`)
case SearchOperator.GreaterThanOrEqualTo:
return semver.gte(coercedVersionOne, coercedVersionTwo)
return semver.satisfies(coercedVersionToCompare, `>=${inputVersion}`)
case SearchOperator.LessThan:
return semver.satisfies(coercedVersionToCompare, `<${inputVersion}`)
case SearchOperator.LessThanOrEqualTo:
return semver.lte(coercedVersionOne, coercedVersionTwo)
return semver.satisfies(coercedVersionToCompare, `<=${inputVersion}`)
case SearchOperator.NotEquals:
return !semver.eq(coercedVersionOne, coercedVersionTwo)
return !semver.satisfies(coercedVersionToCompare, inputVersion)
default:
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ export function ClustersTable(props: {
clusterImageSets,
agentClusterInstalls,
props.clusters
)?.split(' ')[1]
)
return handleSemverOperatorComparison(clusterVersion ?? '', value, operator)
},
},
Expand Down