Skip to content

feat: display uncommit&unpush change - [INS-4138]#7816

Merged
CurryYangxx merged 10 commits intodevelopfrom
feat/show-uncommit
Aug 12, 2024
Merged

feat: display uncommit&unpush change - [INS-4138]#7816
CurryYangxx merged 10 commits intodevelopfrom
feat/show-uncommit

Conversation

@CurryYangxx
Copy link
Member

@CurryYangxx CurryYangxx commented Aug 7, 2024

image

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:

  • add hasUnpushedChanges and hasUncommittedChanges to the workspace meta model
  • add can-push loader to check if there are unpushed changes
  • check diff with remote when working in workspace and update workspace meta
  • UI: replace the project icon color to mark change

Next step:

  • show inactive project unpush status

@CurryYangxx CurryYangxx marked this pull request as draft August 7, 2024 02:27
@CurryYangxx CurryYangxx changed the title WIP: display uncommit WIP: display uncommit - [INS-4138] Aug 7, 2024
@CurryYangxx CurryYangxx force-pushed the feat/show-uncommit branch 2 times, most recently from 119e9cb to 0854418 Compare August 7, 2024 08:08
@CurryYangxx CurryYangxx changed the title WIP: display uncommit - [INS-4138] WIP: display uncommit&unpush change - [INS-4138] Aug 7, 2024
@CurryYangxx CurryYangxx requested a review from gatzjames August 7, 2024 10:39
@CurryYangxx CurryYangxx marked this pull request as ready for review August 7, 2024 10:51
@subnetmarco
Copy link
Member

We need to show the dot in the cards as well:

Screenshot 2024-08-07 at 9 36 23 AM

@CurryYangxx CurryYangxx changed the title WIP: display uncommit&unpush change - [INS-4138] feat: display uncommit&unpush change - [INS-4138] Aug 8, 2024
@CurryYangxx
Copy link
Member Author

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, {
Copy link
Contributor

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

Copy link
Member Author

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.

Comment on lines +342 to +348
// 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,
},
});
Copy link
Contributor

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:

Suggested change
// 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,
},
});

Copy link
Member Author

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👍

@CurryYangxx CurryYangxx merged commit 76976db into develop Aug 12, 2024
@CurryYangxx CurryYangxx deleted the feat/show-uncommit branch August 12, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants