-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: display uncommit&unpush change - [INS-4138] #7816
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
Conversation
119e9cb to
0854418
Compare
4e4411f to
c04c7f3
Compare
|
updated |
| const { canPush } = gitCanPushFetcher.data; | ||
| const { changes } = gitChangesFetcher.data; | ||
| // update workspace meta with git sync data, use for show unpushed changes on collection card | ||
| models.workspaceMeta.updateByParentId(workspaceId, { |
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 think this should go to the loaders/actions that it relies upon.
My thinking:
- We want to move db calls out of the UI in general
- It's going to remove an extra useEffect
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.
Move them to the loaders&actions.
FYI: I removed syncData and put hasUncommittedChanges and hasUnpushedChanges directly on the first level. Because the db update will not deep merge an object.
| // update workspace meta with sync data, use for show unpushed changes on collection card | ||
| models.workspaceMeta.updateByParentId(workspaceId, { | ||
| syncData: { | ||
| hasUncommittedChanges: Object.keys(status?.unstaged || {}).length > 0 || Object.keys(status?.stage || {}).length > 0, | ||
| hasUnpushedChanges: compare?.ahead > 0, | ||
| }, | ||
| }); |
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.
Thinking this is simpler to understand. What do you think? Feel free to add/dismiss this one:
| // update workspace meta with sync data, use for show unpushed changes on collection card | |
| models.workspaceMeta.updateByParentId(workspaceId, { | |
| syncData: { | |
| hasUncommittedChanges: Object.keys(status?.unstaged || {}).length > 0 || Object.keys(status?.stage || {}).length > 0, | |
| hasUnpushedChanges: compare?.ahead > 0, | |
| }, | |
| }); | |
| let hasUncommittedChanges = false; | |
| if (status?.unstaged && Object.keys(status.unstaged).length > 0) { | |
| hasUncommittedChanges = true; | |
| } | |
| if (status?.stage && Object.keys(stage).length > 0) { | |
| hasUncommittedChanges = true; | |
| } | |
| // update workspace meta with sync data, use for show unpushed changes on collection card | |
| models.workspaceMeta.updateByParentId(workspaceId, { | |
| syncData: { | |
| hasUncommittedChanges, | |
| hasUnpushedChanges: compare?.ahead > 0, | |
| }, | |
| }); |
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.
Good advice. I agree with you👍
c94d763 to
03b9eea
Compare
26599c5 to
e96e235
Compare

Feature: Show uncommitted & unpushed change for both insomnia sync and git sync
Currently this pr can only show unpush/uncommit status for active project
Changes:
hasUnpushedChangesandhasUncommittedChangesto the workspace meta modelcan-pushloader to check if there are unpushed changesNext step: