dbeaver/pro#8068 [CB] Fix: form reload on save and AI settings language#4140
Open
SychevAndrey wants to merge 7 commits intodevelfrom
Open
dbeaver/pro#8068 [CB] Fix: form reload on save and AI settings language#4140SychevAndrey wants to merge 7 commits intodevelfrom
SychevAndrey wants to merge 7 commits intodevelfrom
Conversation
…een loaded=false and savingPromise Problem: FormPart.save() sets loaded=false after successful save, expecting useAutoLoad to call load() and reload data from server. However, load() is blocked by formState.savingPromise guard (added recently in dbeaver/pro#8151, 9c406a4), which is only cleared later in FormState.save() finally block. By the time savingPromise is cleared, no observable changes trigger a React re-render, so useAutoLoad never retries — loaded stays false forever and isChanged always returns false, permanently disabling the Save button. Solution: Make FormPart.save() self-contained by adding an inline post-save reload. After saveChanges(), set loaded=false (so setInitialState resets both initialState and state), call loader() directly, then set loaded=true. This eliminates the dependency on the external useAutoLoad mechanism for post-save reload, avoiding the race condition with savingPromise entirely.
When allowCustomValue is enabled, pressing Enter was selecting an auto-highlighted item from the dropdown instead of keeping the user's typed value. Disable autoSelect and close the popover via Ariakit store on Enter to match the blur behavior.
Wroud
requested changes
Feb 16, 2026
Comment on lines
119
to
124
| try { | ||
| await this.loader(); | ||
| this.exception = null; | ||
| } finally { | ||
| this.loaded = true; | ||
| } |
Member
There was a problem hiding this comment.
This should not be here, form will be automatically loaded after saving in another process. Loading is not a part of saving so loading error should not be handled in this place. And loading should not be triggered here
Member
|
You described a bug with previous fix with no proper observable, this can be fixed by modifying one of the loading state interface flags (you can set |
…via setInitialState
Wroud
requested changes
Feb 18, 2026
Member
There was a problem hiding this comment.
I will suggest the next solution:
in the ILoadableState interface:
- add optional field
isLoadable(you might choose a different name, but keep in mind thatCachedResourcehas this field already, so if you will use different name, it's better to renameisLodablein theCachedResourceto that name)
in the useAutoLoad:
- In
getComputed, add a call to theisLoadableto trigger re-render when this flag changes - In the
loadFunctionNamefunction, skip loading the call ifisLoadable?.() === false
in the FormPart class:
- Implement the
isLoadablefunctionreturn !this.formState.savingPromise - Use this new function in the
loadfunction instead ofthis.formState.savingPromise
motivation:
- There might be many parts with heavy data in a single form, we don't want to load all this data until the user switches to the specified part
- We can't just set the current
stateto theinitialStatebecause there are sometimes bugs on the backend side or concurrent collisions resulting in different frontend and backend state
| } | ||
|
|
||
| this.loaded = false; | ||
| this.setInitialState(toJS(this.state)); |
Member
There was a problem hiding this comment.
this is very bad idea, because backend might return different value for state
devnaumov
reviewed
Feb 20, 2026
| getComputed( | ||
| // activate mobx subscriptions | ||
| () => (!loader.isLoaded() || loader.isOutdated?.() === true) && !loader.isError(), | ||
| () => (!loader.isLoaded() || loader.isOutdated?.() === true) && !loader.isError() && loader.isLoadable?.() !== false, |
Member
There was a problem hiding this comment.
loader.isLoadable?.() === true ?
as !== false will include undefined
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Don't autoselect on Enter if allowCustomValue