Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const { RECIPIENT_FIELD } = require('@condo/domains/acquiring/schema/fields/Reci
const { ACQUIRING_INTEGRATION_FIELD } = require('@condo/domains/acquiring/schema/fields/relations')
const { PERCENT_FIELD } = require('@condo/domains/common/schema/fields')
const { normalizeEmail } = require('@condo/domains/common/utils/mail')
const { hasValidJsonStructure } = require('@condo/domains/common/utils/validation.utils')
const { hasRequiredJsonObject } = require('@condo/domains/common/utils/validation.utils')
const { MarketSetting } = require('@condo/domains/marketplace/utils/serverSchema')
const { STATUS_FIELD, getStatusDescription, getStatusResolver } = require('@condo/domains/miniapp/schema/fields/context')

Expand Down Expand Up @@ -72,7 +72,7 @@ const AcquiringIntegrationContext = new GQLListSchema('AcquiringIntegrationConte
isRequired: true,
hooks: {
validateInput: (args) => {
hasValidJsonStructure(args, true, 1, {})
hasRequiredJsonObject(args)
},
},
},
Expand All @@ -84,7 +84,7 @@ const AcquiringIntegrationContext = new GQLListSchema('AcquiringIntegrationConte
isRequired: true,
hooks: {
validateInput: (args) => {
hasValidJsonStructure(args, true, 1, {})
hasRequiredJsonObject(args)
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions apps/condo/domains/billing/schema/BillingAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {
UNEQUAL_CONTEXT_ERROR,
} = require('@condo/domains/common/constants/errors')
const { IMPORT_ID_FIELD, UNIT_TYPE_FIELD } = require('@condo/domains/common/schema/fields')
const { hasValidJsonStructure } = require('@condo/domains/common/utils/validation.utils')
const { hasRequiredJsonObject } = require('@condo/domains/common/utils/validation.utils')

const { RAW_DATA_FIELD } = require('./fields/common')
const { INTEGRATION_CONTEXT_FIELD, BILLING_PROPERTY_FIELD } = require('./fields/relations')
Expand Down Expand Up @@ -91,7 +91,7 @@ const BillingAccount = new GQLListSchema('BillingAccount', {
if (!resolvedData.hasOwnProperty(fieldPath)) return // skip if on value
const value = resolvedData[fieldPath]
if (value === null) return // null is OK
if (!hasValidJsonStructure(args, true, 1, {}))
if (!hasRequiredJsonObject(args))
return addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] ${fieldPath} field type error. We expect JSON Object`)
Comment on lines +94 to 95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid appending a misleading second validation error.

On Line 94-95, hasRequiredJsonObject(args) already records a specific error; adding JSON_EXPECT_OBJECT_ERROR on every false result can produce duplicate or incorrect messaging (notably for dv mismatch cases).

Suggested fix
-                    if (!hasRequiredJsonObject(args))
-                        return addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] ${fieldPath} field type error. We expect JSON Object`)
+                    if (!hasRequiredJsonObject(args)) return
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!hasRequiredJsonObject(args))
return addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] ${fieldPath} field type error. We expect JSON Object`)
if (!hasRequiredJsonObject(args)) return
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/condo/domains/billing/schema/BillingAccount.js` around lines 94 - 95,
The current check calls hasRequiredJsonObject(args) and then unconditionally
prepends JSON_EXPECT_OBJECT_ERROR when adding an error, which causes
duplicate/misleading messages; update the failing branch so it does NOT prepend
JSON_EXPECT_OBJECT_ERROR and instead either rely solely on the error already
recorded by hasRequiredJsonObject(args) or add a single concise message (e.g.,
using fieldPath and "field type error. We expect JSON Object") via
addFieldValidationError; change the code around hasRequiredJsonObject,
addFieldValidationError, JSON_EXPECT_OBJECT_ERROR and fieldPath accordingly so
duplicate/error-prefixing is removed.

},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { find, getById, GQLListSchema } = require('@open-condo/keystone/schema')

const access = require('@condo/domains/billing/access/BillingIntegrationOrganizationContext')
const { validateReport } = require('@condo/domains/billing/utils/validation.utils')
const { hasValidJsonStructure } = require('@condo/domains/common/utils/validation.utils')
const { hasRequiredJsonObject } = require('@condo/domains/common/utils/validation.utils')
const { CONTEXT_FINISHED_STATUS } = require('@condo/domains/miniapp/constants')
const { STATUS_FIELD, getStatusResolver, getStatusDescription } = require('@condo/domains/miniapp/schema/fields/context')
const { ORGANIZATION_OWNED_FIELD } = require('@condo/domains/organization/schema/fields')
Expand Down Expand Up @@ -39,7 +39,7 @@ const BillingIntegrationOrganizationContext = new GQLListSchema('BillingIntegrat
isRequired: true,
hooks: {
validateInput: (args) => {
hasValidJsonStructure(args, true, 1, {})
hasRequiredJsonObject(args)
},
},
},
Expand All @@ -60,7 +60,7 @@ const BillingIntegrationOrganizationContext = new GQLListSchema('BillingIntegrat
isRequired: true,
hooks: {
validateInput: (args) => {
hasValidJsonStructure(args, true, 1, {})
hasRequiredJsonObject(args)
},
},
},
Expand Down
2 changes: 0 additions & 2 deletions apps/condo/domains/common/constants/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const JSON_EXPECT_OBJECT_ERROR = '[json:expectObject:'
const JSON_EXPECT_ARRAY_ERROR = '[json:expectArray:'
const JSON_NO_NULL_ERROR = '[json:noNull:'
const JSON_UNKNOWN_VERSION_ERROR = '[json:unknownDataVersion:'
const JSON_WRONG_VERSION_FORMAT_ERROR = '[json:wrongDataVersionFormat:'
const JSON_SCHEMA_VALIDATION_ERROR = '[json:schemaValidationError:'
const UNIQUE_ALREADY_EXISTS_ERROR = '[unique:alreadyExists:'
const REQUIRED_NO_VALUE_ERROR = '[required:noValue:'
Expand Down Expand Up @@ -84,7 +83,6 @@ module.exports = {
PHONE_WRONG_FORMAT_ERROR,
EMAIL_WRONG_FORMAT_ERROR,
JSON_UNKNOWN_VERSION_ERROR,
JSON_WRONG_VERSION_FORMAT_ERROR,
JSON_EXPECT_OBJECT_ERROR,
JSON_EXPECT_ARRAY_ERROR,
JSON_NO_NULL_ERROR,
Expand Down
66 changes: 45 additions & 21 deletions apps/condo/domains/common/utils/validation.utils.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
const { isNull, isUndefined, isObject } = require('lodash')
const validate = require('validate.js')
/**
* @deprecated
*/

const isNull = require('lodash/isNull')
const isObject = require('lodash/isObject')
const isUndefined = require('lodash/isUndefined')

const {
JSON_WRONG_VERSION_FORMAT_ERROR,
JSON_UNKNOWN_VERSION_ERROR,
JSON_EXPECT_OBJECT_ERROR,
REQUIRED_NO_VALUE_ERROR,
Expand Down Expand Up @@ -56,42 +60,62 @@
return hasOneField
}

function hasValidJsonStructure (args, isRequired, dataVersion, fieldsConstraints) {
// @deprecated
function hasRequiredJsonObject (args) {
const { resolvedData, fieldPath, addFieldValidationError } = args

if (isRequired && !resolvedData.hasOwnProperty(fieldPath)) {
if (!resolvedData.hasOwnProperty(fieldPath)) {
addFieldValidationError(`${REQUIRED_NO_VALUE_ERROR}${fieldPath}] Value is required`)

return false
}

const value = resolvedData[fieldPath]

if (!isObject(value) || isNull(value))
return addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] Expect JSON Object`)
if (!isObject(value) || isNull(value)) {

Check warning on line 75 in apps/condo/domains/common/utils/validation.utils.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'isNull' is deprecated.

See more on https://sonarcloud.io/project/issues?id=open-condo-software_condo&issues=AZ4-_-ngtY4V-6iAnuUa&open=AZ4-_-ngtY4V-6iAnuUa&pullRequest=7626
addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] Expect JSON Object`)

const { dv, ...data } = value
return false
}

if (dv === dataVersion) {
const errors = validate(data, fieldsConstraints)
if (value.dv !== 1) {
addFieldValidationError(`${JSON_UNKNOWN_VERSION_ERROR}${fieldPath}] Unknown \`dv\` attr inside JSON Object`)

if (errors) {
for (const name of Object.keys(errors)) {
for (const err of errors[name]) {
addFieldValidationError(`${JSON_WRONG_VERSION_FORMAT_ERROR}${fieldPath}] Field '${name}': ${err}`)
}
}
}
return false
}

return !errors
} else {
return addFieldValidationError(`${JSON_UNKNOWN_VERSION_ERROR}${fieldPath}] Unknown \`dv\` attr inside JSON Object`)
return true
}

// @deprecated
function hasOptionalJsonObject (args) {
const { resolvedData, fieldPath, addFieldValidationError } = args

if (!resolvedData.hasOwnProperty(fieldPath)) {
return true
}

const value = resolvedData[fieldPath]

if (!isObject(value) || isNull(value)) {

Check warning on line 100 in apps/condo/domains/common/utils/validation.utils.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'isNull' is deprecated.

See more on https://sonarcloud.io/project/issues?id=open-condo-software_condo&issues=AZ4-_-ngtY4V-6iAnuUb&open=AZ4-_-ngtY4V-6iAnuUb&pullRequest=7626
addFieldValidationError(`${JSON_EXPECT_OBJECT_ERROR}${fieldPath}] Expect JSON Object`)

return false
}

if (value.dv !== 1) {
addFieldValidationError(`${JSON_UNKNOWN_VERSION_ERROR}${fieldPath}] Unknown \`dv\` attr inside JSON Object`)

return false
}

return true
}

module.exports = {
JSON_STRUCTURE_FIELDS_CONSTRAINTS,
hasDbFields,
hasOneOfFields,
hasValidJsonStructure,
hasRequiredJsonObject,
hasOptionalJsonObject,
}
165 changes: 106 additions & 59 deletions apps/condo/domains/common/utils/validation.utils.spec.js
Original file line number Diff line number Diff line change
@@ -1,77 +1,124 @@
const { hasValidJsonStructure } = require('./validation.utils')
const {
hasRequiredJsonObject,
hasOptionalJsonObject,
} = require('./validation.utils')

describe('hasValidJsonStructure()', () => {
describe('dv', () => {
test('no dv', () => {
describe('JSON object validators', () => {
const makeArgs = (fieldPath, value, errors) => ({
resolvedData: { [fieldPath]: value },
fieldPath,
addFieldValidationError: (err) => errors.push(err),
})

describe('hasRequiredJsonObject()', () => {
test('should fail when field is missing and return error', () => {
const errors = []
const fieldPath = 'meta'
const addFieldValidationError = (err) => errors.push(err)
const args = { resolvedData: { [fieldPath]: {} }, fieldPath, addFieldValidationError }
const args = {
resolvedData: {},
fieldPath: 'meta',
addFieldValidationError: (e) => errors.push(e),
}

hasValidJsonStructure(args, true, 1, {})
const result = hasRequiredJsonObject(args)

expect(result).toBe(false)
expect(errors).toEqual([
'[json:unknownDataVersion:meta] Unknown `dv` attr inside JSON Object',
'[required:noValue:meta] Value is required',
])
})
test('wrong dv', () => {

test('should fail when value is not object and return error', () => {
const errors = []
const args = makeArgs('meta', 'string', errors)

const result = hasRequiredJsonObject(args)

expect(result).toBe(false)
expect(errors).toEqual([
'[json:expectObject:meta] Expect JSON Object',
])
})

test('should fail when dv is wrong and return error', () => {
const errors = []
const fieldPath = 'meta'
const addFieldValidationError = (err) => errors.push(err)
const args = { resolvedData: { [fieldPath]: { dv: 2 } }, fieldPath, addFieldValidationError }
const args = makeArgs('meta', { dv: 2 }, errors)

hasValidJsonStructure(args, false, 1, {})
const result = hasRequiredJsonObject(args)

expect(result).toBe(false)
expect(errors).toEqual([
'[json:unknownDataVersion:meta] Unknown `dv` attr inside JSON Object',
])
})

test('should pass when object is valid and return true', () => {
const errors = []
const args = makeArgs('meta', { dv: 1, name: 'test' }, errors)

const result = hasRequiredJsonObject(args)

expect(result).toBe(true)
expect(errors).toEqual([])
})
})

describe('fieldConstrains', () => {
describe('string type check', () => {
test('for no value', () => {
const errors = []
const fieldPath = 'meta'
const addFieldValidationError = (err) => errors.push(err)
const args = { resolvedData: { [fieldPath]: { dv: 1 } }, fieldPath, addFieldValidationError }

hasValidJsonStructure(args, false, 1, {
name: { type: 'string' },
})

expect(errors).toEqual([])
})
test('for empty value', () => {
const errors = []
const fieldPath = 'meta'
const addFieldValidationError = (err) => errors.push(err)
const args = { resolvedData: { [fieldPath]: { dv: 1, name: '' } }, fieldPath, addFieldValidationError }

hasValidJsonStructure(args, false, 1, {
name: { type: 'string' },
})

expect(errors).toEqual([])
})
test('for number value', () => {
const errors = []
const fieldPath = 'meta'
const addFieldValidationError = (err) => errors.push(err)
const args = {
resolvedData: { [fieldPath]: { dv: 1, name: 3221 } },
fieldPath,
addFieldValidationError,
}

hasValidJsonStructure(args, false, 1, {
name: { type: 'string' },
})

expect(errors).toEqual([
'[json:wrongDataVersionFormat:meta] Field \'name\': Name must be of type string',
])
})
describe('hasOptionalJsonObject()', () => {
test('should pass when field is missing and return true', () => {
const errors = []
const args = {
resolvedData: {},
fieldPath: 'meta',
addFieldValidationError: (e) => errors.push(e),
}

const result = hasOptionalJsonObject(args)

expect(result).toBe(true)
expect(errors).toEqual([])
})

test('should fail when value is not object and return error', () => {
const errors = []
const args = makeArgs('meta', 123, errors)

const result = hasOptionalJsonObject(args)

expect(result).toBe(false)
expect(errors).toEqual([
'[json:expectObject:meta] Expect JSON Object',
])
})

test('should fail when dv is wrong and return error', () => {
const errors = []
const args = makeArgs('meta', { dv: 3 }, errors)

const result = hasOptionalJsonObject(args)

expect(result).toBe(false)
expect(errors).toEqual([
'[json:unknownDataVersion:meta] Unknown `dv` attr inside JSON Object',
])
})

test('should pass when object is valid and return true', () => {
const errors = []
const args = makeArgs('meta', { dv: 1 }, errors)

const result = hasOptionalJsonObject(args)

expect(result).toBe(true)
expect(errors).toEqual([])
})

test('should pass when object has extra fields and return true', () => {
const errors = []
const args = makeArgs('meta', { dv: 1, foo: 'bar' }, errors)

const result = hasOptionalJsonObject(args)

expect(result).toBe(true)
expect(errors).toEqual([])
})
})
})
})
Loading
Loading