Skip to content

Conversation

@Shahbaz-dataq
Copy link
Contributor

External References

@Shahbaz-dataq
Copy link
Contributor Author

Waiting for this PR to merge.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR integrates an API for creating and updating event configuration sources. The main change corrects a typo in the store method name from refreshSourcesfilters to refreshSourcesFilters and implements a new dialog component for creating event configuration sources with proper form validation, error handling, and success state management.

Key changes:

  • Fixed method name typo: refreshSourcesfiltersrefreshSourcesFilters
  • Added addEventConfigSource service function with HTTP error handling (400, 409, 500 status codes)
  • Enhanced CreateEventConfigurationDialog with vendor and description fields, success state UI, and navigation to created sources

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ui/src/stores/eventConfigStore.ts Corrected method name typo in store
ui/src/services/eventConfigService.ts Added new service function for creating event configuration sources
ui/src/components/EventConfiguration/Dialog/CreateEventConfigurationDialog.vue Enhanced dialog with additional fields, success state, and navigation
ui/src/components/EventConfiguration/EventConfigSourceTable.vue Updated method call to use corrected name
ui/src/components/EventConfiguration/Dialog/DeleteEventConfigSourceDialog.vue Updated method call to use corrected name
ui/tests/stores/eventConfigStore.test.ts Updated test to use corrected method name
ui/tests/components/EventConfiguration/EventConfigSourceTable.test.ts Updated test to use corrected method name
ui/tests/components/EventConfiguration/Dialog/CreateEventConfigurationDialog.test.ts Added comprehensive test coverage for new dialog functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

import { FeatherDialog } from '@featherds/dialog'
import { FeatherInput } from '@featherds/input'
import { FeatherTextarea } from '@featherds/textarea'
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Missing import for useRouter and ref/Ref/computed from Vue. The component references useRouter(), ref(), Ref, and computed() but these are not imported in the script setup block.

Suggested change
import { ref, Ref, computed } from 'vue'
import { useRouter } from 'vue-router'

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +39
const mockSuccessResponse = (id: number, name: string, fileOrder: number = 0) => ({
id,
name,
fileOrder,
status: 201 as const
})
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The helper function mockSuccessResponse duplicates the response structure defined in the service. Consider extracting this to a shared test utility to maintain consistency across test files if this pattern is used elsewhere.

Copilot uses AI. Check for mistakes.
@Shahbaz-dataq Shahbaz-dataq marked this pull request as ready for review December 17, 2025 16:56
Copy link
Contributor

@synqotik synqotik left a comment

Choose a reason for hiding this comment

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

Added a few comments.

;(wrapper.vm as any).vendor = 'TestVendor'
;(addEventConfigSource as any).mockResolvedValue(mockSuccessResponse(123, 'TestConfig', 0))

await wrapper.vm.$nextTick()
Copy link
Contributor

Choose a reason for hiding this comment

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

These 4 lines are called a bunch of times. Consider moving them to a function called clickCreateButton, then you can call:

await clickCreateButton(wrapper)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing setWrapperRefs. This also needs to be fixed (clickCreateButton).

@Shahbaz-dataq Shahbaz-dataq requested a review from synqotik January 5, 2026 10:24
Copy link
Contributor

@synqotik synqotik left a comment

Choose a reason for hiding this comment

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

See comment, please add something like clickCreateButton in the tests to cut down on duplicate code.

A note, in the future, can use jest parameterized tests for cases where we test a lot of similar things.

;(wrapper.vm as any).vendor = 'TestVendor'
;(addEventConfigSource as any).mockResolvedValue(mockSuccessResponse(123, 'TestConfig', 0))

await wrapper.vm.$nextTick()
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing setWrapperRefs. This also needs to be fixed (clickCreateButton).

Copy link
Contributor

@synqotik synqotik left a comment

Choose a reason for hiding this comment

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

LGTM

@Shahbaz-dataq Shahbaz-dataq merged commit 398a46d into release-35.x Jan 5, 2026
6 checks passed
@Shahbaz-dataq Shahbaz-dataq deleted the jira/NMS-19161-create-source-api-integration branch January 5, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants