-
Notifications
You must be signed in to change notification settings - Fork 2
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
Using update_only
when doing background discover and content change
#955
Changes from 5 commits
28fc769
f713c1c
42cfdae
54dc70a
8f9b44d
b594007
06bcf39
4ea898c
58006b5
4b9cbb7
cc33013
6981433
5deaf49
fe268ea
65e4c67
22c6b86
046a56c
d47464a
d995543
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,14 @@ | ||
import { Button } from '@mui/material'; | ||
import { buttonSx } from 'components/shared/Entity/Header'; | ||
import { useEntityWorkflow_Editing } from 'context/Workflow'; | ||
import { Dispatch, SetStateAction, useEffect } from 'react'; | ||
import { FormattedMessage } from 'react-intl'; | ||
import { | ||
useDetailsForm_connectorImage_connectorId, | ||
useDetailsForm_entityNameChanged, | ||
useDetailsForm_previousConnectorImage_connectorId, | ||
} from 'stores/DetailsForm/hooks'; | ||
import { useEndpointConfigStore_changed } from 'stores/EndpointConfig/hooks'; | ||
import { useFormStateStore_status } from 'stores/FormState/hooks'; | ||
import { FormStatus } from 'stores/FormState/types'; | ||
import { useResourceConfig_rediscoveryRequired } from 'stores/ResourceConfig/hooks'; | ||
|
@@ -27,10 +29,18 @@ function CaptureGenerateButton({ | |
disabled, | ||
createWorkflowMetadata, | ||
}: Props) { | ||
const isEdit = useEntityWorkflow_Editing(); | ||
const endpointConfigChanged = useEndpointConfigStore_changed(); | ||
const rediscoveryRequired = useResourceConfig_rediscoveryRequired(); | ||
|
||
const { generateCatalog, isSaving, formActive } = useDiscoverCapture( | ||
entityType, | ||
{ | ||
// We only want to set updateOnly if the user is editing and not updating the config | ||
// This should cover when a user has enable previously disabled collection(s) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Removing the passive voice from this comment will increase its clarity: |
||
updateOnly: Boolean( | ||
rediscoveryRequired && isEdit && !endpointConfigChanged() | ||
), | ||
initiateDiscovery: createWorkflowMetadata?.initiateDiscovery, | ||
initiateRediscovery: rediscoveryRequired, | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import { | ||
useEditorStore_persistedDraftId, | ||
useEditorStore_setCatalogName, | ||
useEditorStore_setId, | ||
} from 'components/editor/Store/hooks'; | ||
import useEntityWorkflowHelpers from 'components/shared/Entity/hooks/useEntityWorkflowHelpers'; | ||
import { useCallback } from 'react'; | ||
|
@@ -19,6 +20,7 @@ | |
useDiscoverStartSubscription(entityType); | ||
const { callFailed } = useEntityWorkflowHelpers(); | ||
|
||
const setDraftId = useEditorStore_setId(); | ||
const persistedDraftId = useEditorStore_persistedDraftId(); | ||
const setCatalogName = useEditorStore_setCatalogName(); | ||
|
||
|
@@ -32,7 +34,8 @@ | |
async ( | ||
processedEntityName: string, | ||
encryptedEndpointConfigResponse: any, | ||
rediscover?: boolean | ||
rediscover?: boolean, | ||
updateOnly?: boolean | ||
) => { | ||
// If we are doing a rediscovery and we have a draft then go ahead and use that draft | ||
// that way the most recent changes to bindings and endpoints will get added to the draft before rediscovery | ||
|
@@ -64,9 +67,16 @@ | |
processedEntityName, | ||
encryptedEndpointConfigResponse, | ||
imageConnectorTagId, | ||
newDraftId | ||
newDraftId, | ||
updateOnly | ||
); | ||
|
||
if (discoverResponse.error) { | ||
// If we failed at discovery we need to clear draft ID like we do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is a little muddled to me. What do you think of the following: // If discovery fails, clear the draft ID to preserve the _Next_ CTA and allow the user to attempt to discover another time. |
||
// when we createDiscoversSubscription. Otherwise, the Test|Save | ||
// buttons will appear. | ||
setDraftId(null); | ||
|
||
callFailed({ | ||
error: { | ||
title: 'captureCreate.generate.failedErrorTitle', | ||
|
@@ -88,7 +98,7 @@ | |
|
||
return true; | ||
}, | ||
[ | ||
callFailed, | ||
createDiscoversSubscription, | ||
endpointConfigData, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the presence of the
useResourceConfig_rediscoveryRequired
hook above, it should be clear that the resource config is what is being referred to by the term config. However, it could not hurt to explicitly state thatupdateOnly
should be set if the user (clicked the Next CTA and) did not change the resource config in an edit workflow.