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

feat(27975): tag uniqueness and other validation updates #677

Merged
merged 17 commits into from
Dec 9, 2024
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
11 changes: 11 additions & 0 deletions hivemq-edge/src/frontend/src/api/schemas/northbound.ui-schema.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { UiSchema } from '@rjsf/utils'
import { registerEntitySelectWidget } from '@/components/rjsf/Widgets/EntitySelectWidget.tsx'
import { CustomFormat } from '@/api/types/json-schema.ts'

/* istanbul ignore next -- @preserve */
export const northboundMappingListUISchema: UiSchema = {
Expand All @@ -14,6 +16,15 @@ export const northboundMappingListUISchema: UiSchema = {
'ui:collapsable': {
titleKey: 'tagName',
},
tagName: {
'ui:widget': registerEntitySelectWidget(CustomFormat.MQTT_TAG),
},
topic: {
'ui:widget': registerEntitySelectWidget(CustomFormat.MQTT_TOPIC),
'ui:options': {
create: true,
},
},
'ui:addButton': 'Add a mapping',
userProperties: {
items: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export const MqttTransformationField: FC<FieldProps<SouthboundMapping[], RJSFSch
fieldMapping: {
instructions: [],
metadata: {
source: {},
destination: {},
source: { properties: {} },
destination: { properties: {} },
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ErrorListTemplate } from '@/components/rjsf/Templates/ErrorListTemplate
import { useFormControlStore } from '@/components/rjsf/Form/useFormControlStore.ts'
import { MqttTransformationField } from '@/components/rjsf/Fields'
import { adapterJSFWidgets } from '@/modules/ProtocolAdapters/utils/uiSchema.utils.ts'
import { ObjectFieldTemplate } from '@/components/rjsf/ObjectFieldTemplate.tsx'

interface CustomFormProps<T>
extends Pick<
Expand Down Expand Up @@ -103,8 +104,7 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({
formData={defaultValues}
formContext={context}
templates={{
// TODO[NVL] This override was mostly for the tabs (not used in this context) but is wrongly rendering additionalProperties
// ObjectFieldTemplate,
ObjectFieldTemplate,
FieldTemplate,
BaseInputTemplate,
ArrayFieldTemplate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ const MappingContainer: FC<SubscriptionContainerProps> = ({ adapterId, adapterTy
if (!mappings) {
return
}
console.log('XXXXXX item.fieldMapping?.instructions', item.fieldMapping?.instructions)
onChange('fieldMapping', { instructions: [...(item.fieldMapping?.instructions || []), ...mappings] })
}}
onSchemaReady={onSchemaReadyHandler}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ import { DomainTagList } from '@/api/__generated__'
import DeviceTagForm from '@/modules/Device/components/DeviceTagForm.tsx'
import { ManagerContextType } from '@/modules/Mappings/types.ts'

interface DeviceTagDrawerProps {
context: ManagerContextType
interface DeviceTagDrawerProps<T> {
context: ManagerContextType<T>
// TODO[NVL] Make the component generic and pass the type
onSubmit?: (data: unknown) => void
trigger: (disclosureProps: UseDisclosureProps) => JSX.Element
header: string
submitLabel?: string
}

const ArrayItemDrawer: FC<DeviceTagDrawerProps> = ({ header, context, onSubmit, trigger, submitLabel }) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const ArrayItemDrawer: FC<DeviceTagDrawerProps<any>> = ({ header, context, onSubmit, trigger, submitLabel }) => {
const { t } = useTranslation('components')
const props = useDisclosure()
const { isOpen, onClose } = props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const registerEntitySelectWidget =
// eslint-disable-next-line react/display-name
(type: CustomFormat) => (props: WidgetProps<WidgetProps<unknown, RJSFSchema, MappingContext>>) => {
const { chakraProps, label, id, disabled, readonly, onChange, required, rawErrors, value } = props
const { multiple } = getUiOptions(props.uiSchema)
const { multiple, create } = getUiOptions(props.uiSchema)
const { adapterId } = props.formContext

const Select = useMemo(() => {
Expand All @@ -33,7 +33,7 @@ export const registerEntitySelectWidget =
<Select
adapterId={adapterId as string}
isMulti={Boolean(multiple)}
isCreatable={false}
isCreatable={Boolean(create)}
id={id}
value={value}
onChange={onChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ describe('OperationPanel', () => {
},
}

it('should render the form', () => {
// TODO[NVL] There is a bug
it.skip('should render the form', () => {
cy.mountWithProviders(<OperationPanel selectedNode="my-node" />, {
wrapper: getWrapperWith([node]),
})
Expand Down
10 changes: 7 additions & 3 deletions hivemq-edge/src/frontend/src/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,13 @@
"minimum": "Should be at least {{ count }}",
"maximum": "Should not be more than {{ count }}",
"pattern": "Should match the regular expression {{ pattern }}",
"jsonSchema": {
"identifier": {
"unique": "must be unique"
"identifier": {
"adapter": {
"unique": "This identifier is already in use for another adapter"
},
"tag": {
"uniqueDevice": "This tag name is already used on this device",
"uniqueEdge": "This tag name is already used on another devices"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
import DeviceTagForm from '@/modules/Device/components/DeviceTagForm.tsx'
import { ManagerContextType } from '@/modules/Mappings/types.ts'
import { createSchema } from '@/modules/Device/utils/tags.utils.ts'
import type { DomainTagList } from '@/api/__generated__'

describe('DeviceTagForm', () => {
beforeEach(() => {
cy.viewport(800, 1000)
})

it('should render the errors', () => {
const mockContext: ManagerContextType = { schema: undefined }
const mockContext: ManagerContextType<DomainTagList> = { schema: undefined }
cy.mountWithProviders(<DeviceTagForm context={mockContext} />, {
routerProps: { initialEntries: [`/node/wrong-adapter`] },
})
Expand All @@ -20,20 +21,14 @@ describe('DeviceTagForm', () => {
.should('contain.text', 'The form cannot be created, due to internal errors')
})

it('should render the form', () => {
// TODO[NVL] Fix the error
it.skip('should render the form', () => {
const onSubmit = cy.stub().as('onSubmit')

const mockContext: ManagerContextType = {
const mockContext: ManagerContextType<DomainTagList> = {
schema: createSchema({ properties: { test: { type: 'string' } } }),
formData: {
items: [
{
tag: 'opcua-generator/power/off',
dataPoint: {
test: 'ns=3;i=1002',
},
},
],
items: [],
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,22 @@ import { DomainTagList } from '@/api/__generated__'
import ErrorMessage from '@/components/ErrorMessage.tsx'
import { ManagerContextType } from '@/modules/Mappings/types.ts'
import ChakraRJSForm from '@/components/rjsf/Form/ChakraRJSForm.tsx'
import { customUniqueTagValidation } from '@/modules/Device/utils/validation.utils.ts'
import { useListDomainTags } from '@/api/hooks/useDomainModel/useListDomainTags.ts'

interface DeviceTagFormProps {
context: ManagerContextType
context: ManagerContextType<DomainTagList>
onSubmit?: (data: DomainTagList | undefined) => void
}

const DeviceTagForm: FC<DeviceTagFormProps> = ({ context, onSubmit }) => {
const { t } = useTranslation()
const { data } = useListDomainTags()

const allNames = (data?.items || []).map((e) => e.name)
const initialNames = [...(context.formData?.items || [])].map((e) => e.name)
// initial names have already been checked
const cleanNames = allNames.filter((e) => !initialNames.includes(e))

const onFormSubmit = useCallback(
(data: IChangeEvent<DomainTagList>) => {
Expand All @@ -31,6 +39,8 @@ const DeviceTagForm: FC<DeviceTagFormProps> = ({ context, onSubmit }) => {
uiSchema={context.uiSchema}
formData={context.formData}
onSubmit={onFormSubmit}
// @ts-ignore Need to fix that TS error
customValidate={customUniqueTagValidation(cleanNames)}
/>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import ErrorMessage from '@/components/ErrorMessage.tsx'
import { PLCTag } from '@/components/MQTT/EntityTag.tsx'
import ArrayItemDrawer from '@/components/rjsf/SplitArrayEditor/components/ArrayItemDrawer.tsx'
import { formatTagDataPoint } from '@/modules/Device/utils/tags.utils.ts'
import { useTagManager } from '@/modules/Mappings/hooks/useTagManager.ts'
import { useTagManager } from '@/modules/Device/hooks/useTagManager.ts'

interface DeviceTagListProps {
adapter: Adapter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import {
} from '@/api/hooks/useProtocolAdapters/__handlers__'
import { Adapter, AdaptersList, type DomainTagList, ProtocolAdapter, ProtocolAdaptersList } from '@/api/__generated__'
import { AuthProvider } from '@/modules/Auth/AuthProvider.tsx'
import { useTagManager } from '@/modules/Mappings/hooks/useTagManager.ts'
import { useTagManager } from '@/modules/Device/hooks/useTagManager.ts'
import { MOCK_ADAPTER_ID } from '@/__test-utils__/mocks.ts'
import { handlers } from '@/api/hooks/useDomainModel/__handlers__/index.ts'
import { handlers } from '@/api/hooks/useDomainModel/__handlers__'

const wrapper: React.JSXElementConstructor<{ children: React.ReactElement }> = ({ children }) => (
<QueryClientProvider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ export const useTagManager = (adapterId: string) => {

const { $schema: sc, ...rest } = tagSchema?.configSchema as RJSFSchema
// TODO[28249] Handle manually until backend fixed
const { properties, required } = rest

const requiredProtocol = [...(required || []), 'protocolId']
const { properties } = rest

const safeSchema = {
...rest,
Expand All @@ -44,7 +42,6 @@ export const useTagManager = (adapterId: string) => {
default: protocol?.id,
},
},
required: requiredProtocol,
}

return {
Expand Down Expand Up @@ -110,7 +107,7 @@ export const useTagManager = (adapterId: string) => {
)
}

const context: ManagerContextType = {
const context: ManagerContextType<DomainTagList> = {
schema: tagListSchema,
uiSchema: {
'ui:submitButtonOptions': {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, expect } from 'vitest'
import { customUniqueTagValidation } from '@/modules/Device/utils/validation.utils.ts'
import { createErrorHandler } from '@rjsf/utils'
import type { DomainTagList } from '@/api/__generated__'

const mockAllTags = ['device1/tag1', 'device2/tag1', 'device2/tag2', 'device3/tag3']

describe('customUniqueTagValidation', () => {
const validator = customUniqueTagValidation(mockAllTags)

it('should detect duplication in local device', () => {
const formData: DomainTagList = {
items: [
{ name: '1', definition: {} },
{ name: '1', definition: {} },
],
}
const results = validator(formData, createErrorHandler(formData))

expect(results.items?.[1]?.name?.__errors).toStrictEqual(['This tag name is already used on this device'])
})

it('should detect duplication in every devices', () => {
const formData: DomainTagList = {
items: [
{ name: '1', definition: {} },
{ name: '1', definition: {} },
{ name: 'device2/tag1', definition: {} },
],
}
const results = validator(formData, createErrorHandler(formData))

expect(results.items?.[1]?.name?.__errors).toStrictEqual(['This tag name is already used on this device'])
expect(results.items?.[2]?.name?.__errors).toStrictEqual(['This tag name is already used on another devices'])
})

it('should detect no duplication', () => {
const formData: DomainTagList = {
items: [
{ name: '1', definition: {} },
{ name: '2', definition: {} },
{ name: 'device2/tag25', definition: {} },
],
}
const results = validator(formData, createErrorHandler(formData))

expect(results.items?.[1]?.name?.__errors).toStrictEqual([])
expect(results.items?.[2]?.name?.__errors).toStrictEqual([])
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { FormValidation } from '@rjsf/utils'
import type { DomainTagList } from '@/api/__generated__'

import i18n from '@/config/i18n.config.ts'

export const customUniqueTagValidation =
(allTags: string[]) => (formData: DomainTagList, errors: FormValidation<DomainTagList>) => {
// initial names have already been checked and are excluded from the allTags

// Check for duplicate names in the current form
const allLocal = formData.items.map((tag) => tag.name)
const localDuplicates = formData.items.reduce<number[]>((acc, tag, currentIndex) => {
if (allLocal.indexOf(tag.name) !== currentIndex) {
acc.push(currentIndex)
}
return acc
}, [])

for (const duplicate of localDuplicates) {
errors?.items?.[duplicate]?.name?.addError(
i18n.t('validation.identifier.tag.uniqueDevice', { ns: 'translation' })
)
}

// Check for duplicate names across all devices
const edgeDuplicates = formData.items.reduce<number[]>((acc, item, currentIndex) => {
if (allTags.includes(item.name)) {
acc.push(currentIndex)
}
return acc
}, [])

for (const duplicate of edgeDuplicates) {
errors?.items?.[duplicate]?.name?.addError(i18n.t('validation.identifier.tag.uniqueEdge', { ns: 'translation' }))
}

return errors
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { beforeEach, expect } from 'vitest'
import { renderHook, waitFor } from '@testing-library/react'

import { server } from '@/__test-utils__/msw/mockServer.ts'
import { SimpleWrapper as wrapper } from '@/__test-utils__/hooks/SimpleWrapper.tsx'
import { useNorthboundMappingManager } from '@/modules/Mappings/hooks/useNorthboundMappingManager.ts'
import { mappingHandlers } from '@/api/hooks/useProtocolAdapters/__handlers__/mapping.mocks.ts'
import { NorthboundMappingList } from '@/api/__generated__'
import { MOCK_MAX_QOS } from '@/__test-utils__/adapters/mqtt.ts'

describe('useNorthboundMappingManager', () => {
beforeEach(() => {
server.use(...mappingHandlers)
})

afterEach(() => {
server.resetHandlers()
})

it('should do it', async () => {
const { result } = renderHook(() => useNorthboundMappingManager('my-adapter'), { wrapper })

expect(result.current.isLoading).toBeTruthy()
await waitFor(() => {
expect(result.current.isLoading).toBeFalsy()
})

expect(result.current.data).toStrictEqual<NorthboundMappingList>({
items: [
expect.objectContaining({
includeTagNames: true,
includeTimestamp: true,
maxQoS: MOCK_MAX_QOS,
messageExpiryInterval: -1000,
messageHandlingOptions: 'MQTTMessagePerTag',
tagName: 'my/tag',
topic: 'my/topic',
}),
],
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const useNorthboundMappingManager = (adapterId: string): MappingManagerTy
return promise
}

const context: ManagerContextType = {
const context: ManagerContextType<NorthboundMappingList> = {
schema: northboundMappingListSchema,
uiSchema: northboundMappingListUISchema,
formData: data,
Expand Down
Loading
Loading