[sync] fix: preserve select lookup choices on lookupOptions update (T6208)#3603
[sync] fix: preserve select lookup choices on lookupOptions update (T6208)#3603tea-artist wants to merge 1 commit into
Conversation
…6208) Synced from teableio/teable-ee@76cba02 Co-authored-by: Aries X <caoxing9@gmail.com> Co-authored-by: nichenqin <nichenqin@hotmail.com>
|
|
1 similar comment
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a94edc2034
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Prefer request options when present; otherwise fall back to previous | ||
| // options so showAs/formatting survive. prepareLookupField still mirrors | ||
| // source choices via prepareFormattingShowAs. | ||
| options: fieldRo.options ?? oldFieldVo.options, |
There was a problem hiding this comment.
When a lookup's linkFieldId changes from a single-value link to a multi-value link and the client omits options, this fallback feeds the old options into prepareFormattingShowAs. A previously valid single-number showAs such as ring is then retained, but validateFormattingShowAs validates the updated lookup against the multi-number schema and rejects the conversion. The previous changed-lookup path rebuilt from fieldRo without carrying old options, so this blocks legitimate lookup target/cardinality changes unless callers know to send options: {}.
Useful? React with 👍 / 👎.
| // options / sends null while only updating lookupOptions (T6208). | ||
| const mergedLookupOptions = | ||
| newLookupOptions && oldLookupOptions | ||
| ? { ...oldLookupOptions, ...newLookupOptions } |
There was a problem hiding this comment.
When a lookup already has a filter and the client switches linkFieldId/foreignTableId/lookupFieldId without sending a new filter, this shallow merge preserves the old filter on the new lookup target. That regresses the previous changed-target path, which rebuilt from fieldRo and dropped omitted filters, and can leave filters that reference fields from the old foreign table so the converted lookup stays unexpectedly filtered or later queries against invalid field ids.
Useful? React with 👍 / 👎.
|
|
||
| return this.prepareLookupField(tableId, fieldRo); | ||
| return this.prepareLookupField(tableId, { | ||
| ...oldFieldVo, |
There was a problem hiding this comment.
When an errored lookup is repaired by converting it to a valid lookupOptions target, spreading oldFieldVo into the object passed to prepareLookupField carries the old hasError flag forward, and the normal lookup preparation path does not clear it. The transaction may later repair the persisted flag via restoreReference, but the newField object used for the API response and OPERATION_FIELD_CONVERT event still reports hasError: true, so clients can keep showing the field as broken immediately after a successful repair; the previous changed-target path rebuilt from fieldRo and did not carry this stale state.
Useful? React with 👍 / 👎.
3 participants
🔄 Automated sync from EE repository.
2 commit(s) synced since last sync.
Authors
Included commits
Latest source commit: teableio/teable-ee@76cba02
This PR was automatically created by the sync workflow.