Skip to content

Commit

Permalink
chore: git mod - minor fixes (appsmithorg#38563)
Browse files Browse the repository at this point in the history
## Description
- Fixes initial loading state for status
- Includes module instances and source module count
- Fixes discard flow

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12692930817>
> Commit: 6689c8e
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12692930817&attempt=4"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Fri, 10 Jan 2025 08:32:22 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
	- Introduced a new focus on source modules instead of packages
	- Enhanced tracking of module instances and source module changes

- **Bug Fixes**
- Updated status change calculations to more accurately reflect module
modifications
	- Improved handling of artifact identification in discard operations

- **Refactor**
- Renamed and restructured various status-related interfaces and
functions
- Modified selectors and reducers to support new module tracking
approach

- **Chores**
	- Updated type definitions and selector implementations
	- Removed deprecated package-related tracking mechanisms

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
brayn003 authored Jan 10, 2025
1 parent 35a0eda commit 9185254
Show file tree
Hide file tree
Showing 14 changed files with 49 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -272,28 +272,14 @@ function statusThemeTransformer(status: FetchStatusResponseData) {
return tree;
}

function statusPackagesTransformer(status: FetchStatusResponseData) {
const {
modifiedModuleInstances = 0,
modifiedModules = 0,
modifiedPackages = 0,
} = status;
function statusModuleInstancesTransformer(status: FetchStatusResponseData) {
const { modifiedModuleInstances = 0, modifiedSourceModules = 0 } = status;
const tree = [] as StatusTreeStruct[];

if (modifiedPackages > 0) {
tree.push(
createTreeNode({
subject: `${modifiedPackages} package${modifiedPackages > 1 ? "s" : ""}`,
verb: "modified",
type: "package",
}),
);
}

if (modifiedModules > 0) {
if (modifiedSourceModules > 0) {
tree.push(
createTreeNode({
subject: `${modifiedModules} module${modifiedModules > 1 ? "s" : ""}`,
subject: `${modifiedSourceModules} source module${modifiedSourceModules > 1 ? "s" : ""}`,
verb: "modified",
type: "module",
}),
Expand Down Expand Up @@ -323,7 +309,7 @@ export default function applicationStatusTransformer(
...statusJsLibTransformer(status),
...statusSettingsTransformer(status),
...statusThemeTransformer(status),
...statusPackagesTransformer(status),
...statusModuleInstancesTransformer(status),
] as StatusTreeStruct[];

return tree;
Expand Down
2 changes: 1 addition & 1 deletion src/git/components/QuickActions/BranchButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default function BranchButton({
isAutocommitPolling = false,
isBranchPopupOpen = false,
isProtectedMode = false,
isStatusClean = false,
isStatusClean = true,
isTriggerAutocommitLoading = false,
toggleBranchPopup = noop,
}: BranchButtonProps) {
Expand Down
2 changes: 1 addition & 1 deletion src/git/components/QuickActions/QuickActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function QuickActionsView({
isProtectedMode = false,
isPullFailing = false,
isPullLoading = false,
isStatusClean = false,
isStatusClean = true,
isTriggerAutocommitLoading = false,
pull = noop,
statusBehindCount = 0,
Expand Down
6 changes: 4 additions & 2 deletions src/git/components/QuickActions/hooks/useStatusChangeCount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ export const calcStatusChangeCount = (status: FetchStatusResponseData) => {
modifiedDatasources = 0,
modifiedJSLibs = 0,
modifiedJSObjects = 0,
modifiedModules = 0,
modifiedModuleInstances = 0,
modifiedPages = 0,
modifiedQueries = 0,
modifiedSourceModules = 0,
} = status || {};
const themeCount = modified.includes("theme.json") ? 1 : 0;
const settingsCount = modified.includes("application.json") ? 1 : 0;
Expand All @@ -20,10 +21,11 @@ export const calcStatusChangeCount = (status: FetchStatusResponseData) => {
modifiedDatasources +
modifiedJSLibs +
modifiedJSObjects +
modifiedModules +
modifiedPages +
modifiedQueries +
themeCount +
modifiedSourceModules +
modifiedModuleInstances +
settingsCount
);
};
Expand Down
2 changes: 1 addition & 1 deletion src/git/components/QuickActions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function QuickActions() {
const { currentBranch, isBranchPopupOpen, toggleBranchPopup } = useBranches();

const isPullFailing = !!pullError;
const isStatusClean = status?.isClean ?? false;
const isStatusClean = status?.isClean ?? true;
const statusBehindCount = status?.behindCount ?? 0;
const statusChangeCount = useStatusChangeCount(status);

Expand Down
9 changes: 5 additions & 4 deletions src/git/hooks/useDiscard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@ import { useDispatch } from "react-redux";
import useArtifactSelector from "./useArtifactSelector";

export default function useDiscard() {
const { artifactDef } = useGitContext();
const { artifact, artifactDef } = useGitContext();
const artifactId = artifact?.id;
const dispatch = useDispatch();

const discardState = useArtifactSelector(selectDiscardState);

const discard = useCallback(() => {
if (artifactDef) {
dispatch(gitArtifactActions.discardInit({ artifactDef }));
if (artifactDef && artifactId) {
dispatch(gitArtifactActions.discardInit({ artifactDef, artifactId }));
}
}, [artifactDef, dispatch]);
}, [artifactDef, artifactId, dispatch]);

const clearDiscardError = useCallback(() => {
if (artifactDef) {
Expand Down
5 changes: 2 additions & 3 deletions src/git/requests/fetchStatusRequest.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ export interface FetchStatusResponseData {
modifiedDatasources: number;
modifiedJSLibs: number;
modifiedJSObjects: number;
modifiedPackages: number;
modifiedModuleInstances: number;
modifiedModules: number;
modifiedPages: number;
modifiedQueries: number;
modifiedSourceModules: number;
modifiedModuleInstances: number;
pagesAdded: string[];
pagesModified: string[];
pagesRemoved: string[];
Expand Down
9 changes: 6 additions & 3 deletions src/git/sagas/discardSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@ import { builderURL } from "ee/RouteBuilder";
import { createMessage, DISCARD_SUCCESS } from "ee/constants/messages";
import discardRequest from "git/requests/discardRequest";
import type { DiscardResponse } from "git/requests/discardRequest.types";
import type { DiscardInitPayload } from "git/store/actions/discardActions";
import { gitArtifactActions } from "git/store/gitArtifactSlice";
import type { GitArtifactPayloadAction } from "git/store/types";
import log from "loglevel";
import { call, delay, put } from "redux-saga/effects";
import { validateResponse } from "sagas/ErrorSagas";

export default function* discardSaga(action: GitArtifactPayloadAction) {
const { artifactDef } = action.payload;
export default function* discardSaga(
action: GitArtifactPayloadAction<DiscardInitPayload>,
) {
const { artifactDef, artifactId } = action.payload;

let response: DiscardResponse | undefined;

try {
response = yield call(discardRequest, artifactDef.baseArtifactId);
response = yield call(discardRequest, artifactId);
const isValidResponse: boolean = yield validateResponse(response);

if (response && isValidResponse) {
Expand Down
21 changes: 15 additions & 6 deletions src/git/store/actions/discardActions.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { createArtifactAction } from "../helpers/createArtifactAction";
import type { GitArtifactErrorPayloadAction } from "../types";
import type {
GitArtifactBasePayload,
GitArtifactErrorPayloadAction,
} from "../types";

export const discardInitAction = createArtifactAction((state) => {
state.apiResponses.discard.loading = true;
state.apiResponses.discard.error = null;
export interface DiscardInitPayload extends GitArtifactBasePayload {
artifactId: string;
}

return state;
});
export const discardInitAction = createArtifactAction<DiscardInitPayload>(
(state) => {
state.apiResponses.discard.loading = true;
state.apiResponses.discard.error = null;

return state;
},
);

export const discardSuccessAction = createArtifactAction((state) => {
state.apiResponses.discard.loading = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const TitleText = styled(Text)`
export enum StaticChangeKind {
SETTINGS = "SETTINGS",
THEME = "THEME",
PACKAGES = "PACKAGES",
MODULES = "MODULES",
REMOTE_AHEAD = "REMOTE_AHEAD",
REMOTE_BEHIND = "REMOTE_BEHIND",
Expand Down Expand Up @@ -56,17 +55,10 @@ const allStaticChangeDefs: Record<
message: "Theme modified",
iconName: "sip-line",
}),
[StaticChangeKind.PACKAGES]: (status: GitStatusData) => ({
condition: (status.modifiedPackages ?? 0) > 0,
message: `${status.modifiedPackages ?? 0} ${
(status.modifiedPackages ?? 0) > 0 ? "packages" : "package"
} modified`,
iconName: "package",
}),
[StaticChangeKind.MODULES]: (status: GitStatusData) => ({
condition: (status.modifiedModules ?? 0) > 0,
message: `${status.modifiedModules ?? 0} ${
(status.modifiedModules ?? 0) > 0 ? "modules" : "module"
condition: (status.modifiedSourceModules ?? 0) > 0,
message: `${status.modifiedSourceModules ?? 0} ${
(status.modifiedSourceModules ?? 0) > 0 ? "modules" : "module"
} modified`,
iconName: "package",
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,39 +342,10 @@ describe("GitChangesList", () => {
expect(getByTestId("t--status-change-THEME")).toBeInTheDocument();
});

it("should render Package related changes", () => {
const store = getMockStore({
gitStatus: {
modifiedPackages: 2,
},
});

const { getByTestId, queryByTestId } = render(
<Provider store={store}>
<GitChangesList />
</Provider>,
);

expect(
queryByTestId("t--status-change-DATASOURCES"),
).not.toBeInTheDocument();
expect(queryByTestId("t--status-change-JSLIBS")).not.toBeInTheDocument();
expect(
queryByTestId("t--status-change-REMOTE_AHEAD"),
).not.toBeInTheDocument();
expect(
queryByTestId("t--status-change-REMOTE_BEHIND"),
).not.toBeInTheDocument();
expect(getByTestId("t--status-change-PACKAGES")).toBeInTheDocument();
expect(queryByTestId("t--status-change-MODULES")).not.toBeInTheDocument();
expect(queryByTestId("t--status-change-SETTINGS")).not.toBeInTheDocument();
expect(queryByTestId("t--status-change-THEME")).not.toBeInTheDocument();
});

it("should render Module related changes", () => {
const store = getMockStore({
gitStatus: {
modifiedModules: 2,
modifiedSourceModules: 2,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export default function GitChangesList() {
<ExpandableChange kind={ExpandableChangeKind.JSLIBS} status={status} />
<StaticChange kind={StaticChangeKind.SETTINGS} status={status} />
<StaticChange kind={StaticChangeKind.THEME} status={status} />
<StaticChange kind={StaticChangeKind.PACKAGES} status={status} />
<StaticChange kind={StaticChangeKind.MODULES} status={status} />
{status?.migrationMessage ? (
<CalloutContainer>
Expand Down
3 changes: 1 addition & 2 deletions src/reducers/uiReducers/gitSyncReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,7 @@ export interface GitStatusData {
modifiedJSObjects: number;
modifiedQueries: number;
modifiedJSLibs: number;
modifiedPackages?: number;
modifiedModules?: number;
modifiedSourceModules?: number;
modifiedModuleInstances?: number;
}

Expand Down
8 changes: 4 additions & 4 deletions src/selectors/gitSyncSelectors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,10 @@ export const getCountOfChangesToCommit = (state: AppState) => {
modifiedDatasources = 0,
modifiedJSLibs = 0,
modifiedJSObjects = 0,
modifiedModules = 0,
modifiedPackages = 0,
modifiedModuleInstances = 0,
modifiedPages = 0,
modifiedQueries = 0,
modifiedSourceModules = 0,
} = gitStatus || {};
const themeCount = modified.includes("theme.json") ? 1 : 0;
const settingsCount = modified.includes("application.json") ? 1 : 0;
Expand All @@ -159,8 +159,8 @@ export const getCountOfChangesToCommit = (state: AppState) => {
modifiedDatasources +
modifiedJSLibs +
modifiedJSObjects +
modifiedModules +
modifiedPackages +
modifiedSourceModules +
modifiedModuleInstances +
modifiedPages +
modifiedQueries +
themeCount +
Expand Down

0 comments on commit 9185254

Please sign in to comment.