-
Notifications
You must be signed in to change notification settings - Fork 24
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
Sync dataset name with datasource in add remote view #8245
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
e8afa5e
sync dataset name with datasource in add remote view
7c6a5e8
add changelog entry
9739183
make syncing simple to advanced tab more robust
09a7486
add comment explaining when syncing datasetNames is needed and when not
17c08db
use TS nullish coalescing operator instead of if
13eb34b
make missing occurences of syncDataSourceFields also sync dataset name
4e51269
Merge branch 'master' into fix-add-remote
MichaelBuessemeyer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,12 +33,16 @@ import { type APIDataLayer, type APIDataset, APIJobType } from "types/api_flow_t | |
import { useStartAndPollJob } from "admin/job/job_hooks"; | ||
import { AllUnits, LongUnitToShortUnitMap, type Vector3 } from "oxalis/constants"; | ||
import Toast from "libs/toast"; | ||
import type { ArbitraryObject } from "types/globals"; | ||
|
||
const FormItem = Form.Item; | ||
|
||
export const syncDataSourceFields = ( | ||
form: FormInstance, | ||
syncTargetTabKey: "simple" | "advanced", | ||
// Syncing the dataset name is optional as this is needed for the add remote view, but not for the edit view. | ||
// In the edit view, the datasource.id fields should never be changed and the backend will automatically ignore all changes to the id field. | ||
syncDatasetName = false, | ||
philippotto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
): void => { | ||
if (!form) { | ||
return; | ||
|
@@ -47,12 +51,25 @@ export const syncDataSourceFields = ( | |
if (syncTargetTabKey === "advanced") { | ||
// Copy from simple to advanced: update json | ||
const dataSourceFromSimpleTab = form.getFieldValue("dataSource"); | ||
if (syncDatasetName && dataSourceFromSimpleTab) { | ||
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. The |
||
dataSourceFromSimpleTab.id ??= {}; | ||
dataSourceFromSimpleTab.id.name = form.getFieldValue(["dataset", "name"]); | ||
} | ||
form.setFieldsValue({ | ||
dataSourceJson: jsonStringify(dataSourceFromSimpleTab), | ||
}); | ||
} else { | ||
const dataSourceFromAdvancedTab = parseMaybe(form.getFieldValue("dataSourceJson")); | ||
const dataSourceFromAdvancedTab = parseMaybe( | ||
form.getFieldValue("dataSourceJson"), | ||
) as ArbitraryObject | null; | ||
// Copy from advanced to simple: update form values | ||
if (syncDatasetName && dataSourceFromAdvancedTab?.id?.name) { | ||
form.setFieldsValue({ | ||
dataset: { | ||
name: dataSourceFromAdvancedTab.id.name, | ||
}, | ||
}); | ||
} | ||
form.setFieldsValue({ | ||
dataSource: dataSourceFromAdvancedTab, | ||
}); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I missed a few occurrences of
syncDataSourceFields
🙈, this one here being the most important.