chore: git tag - fixing contracts for pretag #39581
Conversation
|
Warning Rate limit exceeded@brayn003 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request migrates commit data fetching from the old "latestCommit" mechanism to a new "pretag" system. It updates multiple React components to use the Changes
Sequence Diagram(s)sequenceDiagram
participant C as TabRelease Component
participant H as usePretag Hook
participant A as Redux Actions
participant API as pretagRequest API
participant S as Redux Store
C->>H: Invoke fetchPretag (via useEffect)
H->>A: Dispatch pretagInitAction
A->>API: Call pretagRequest()
API-->>A: Return PretagResponse
A->>S: Dispatch pretagSuccessAction with response
S-->>H: Update pretag state
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Git Product
github-actions
bot
added
the
skip-changelog
brayn003
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/client/src/git/components/LatestCommitInfo/index.tsx (1)
10-13: Property access paths updated for new structure.All property access paths have been correctly updated to match the new
pretagResponsestructure. Consider adding extra null check for the author property.- authorName={pretagResponse?.author.name ?? null} + authorName={pretagResponse?.author?.name ?? null}app/client/src/git/requests/pretagRequest.types.ts (1)
3-15: Interface structure is well-defined.The
PretagResponseDatainterface clearly defines the expected structure of the pretag response data. Consider adding JSDoc comments for better documentation.+/** + * Represents the data structure of a pretag response + */ export interface PretagResponseData { + /** + * Author information of the commit + */ author: { name: string; email: string; }; + /** + * ISO timestamp of when the commit was made + */ committedAt: string; + /** + * Commit hash identifier + */ hash: string; + /** + * Commit message + */ message: string; + /** + * Name of the release tag + */ releaseTagName: string; + /** + * ISO timestamp of when the release was created + */ releasedAt: string; + /** + * Indicates if the commit can be released + */ isReleasable: boolean; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
app/client/src/git/components/LatestCommitInfo/index.tsx(1 hunks)app/client/src/git/components/OpsModal/TabRelease/TabReleaseView.tsx(3 hunks)app/client/src/git/components/OpsModal/TabRelease/index.tsx(1 hunks)app/client/src/git/components/ReleaseVersionRadioGroup/index.tsx(2 hunks)app/client/src/git/hooks/useLatestCommit.ts(0 hunks)app/client/src/git/hooks/usePretag.ts(1 hunks)app/client/src/git/requests/fetchLatestCommitRequest.types.ts(0 hunks)app/client/src/git/requests/pretagRequest.ts(1 hunks)app/client/src/git/requests/pretagRequest.types.ts(1 hunks)app/client/src/git/store/actions/fetchLatestCommitActions.ts(0 hunks)app/client/src/git/store/actions/pretagActions.ts(1 hunks)app/client/src/git/store/gitArtifactSlice.ts(2 hunks)app/client/src/git/store/helpers/initialState.ts(1 hunks)app/client/src/git/store/selectors/gitArtifactSelectors.ts(1 hunks)app/client/src/git/store/types.ts(2 hunks)
💤 Files with no reviewable changes (3)
- app/client/src/git/requests/fetchLatestCommitRequest.types.ts
- app/client/src/git/hooks/useLatestCommit.ts
- app/client/src/git/store/actions/fetchLatestCommitActions.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
🔇 Additional comments (26)
app/client/src/git/hooks/usePretag.ts (5)
1-7: Imports are correctly organized.The imports are well-structured and include all necessary dependencies for the hook functionality.
8-12: Well-structured hook with proper context extraction.The hook correctly uses the Git context to extract necessary artifact information.
13-14: State selection implemented correctly.The hook properly uses the selector to retrieve the pretag state from the Redux store.
15-19: Memoized callback with appropriate dependencies.The
fetchPretagfunction is correctly memoized withuseCallbackand includes all the necessary dependencies in the dependency array.
21-26: Return object handles null cases properly.The return object correctly uses the null coalescing operator to provide fallback values when the state properties are undefined.
app/client/src/git/components/OpsModal/TabRelease/index.tsx (3)
1-4: Imports are correctly structured.The component imports all necessary dependencies including the new
usePretaghook.
5-9: Component implementation is concise and focused.The component has a single responsibility - to connect the pretag functionality to the view component.
11-12: Export is properly defined.The default export makes the component available for import in other modules.
app/client/src/git/components/LatestCommitInfo/index.tsx (2)
3-3: Import updated to use the new hook.Correctly updated to use
usePretaginstead of the previoususeLatestCommithook.
6-6: Destructuring updated for new data structure.Correctly updated to extract
pretagResponseinstead oflatestCommit.app/client/src/git/requests/pretagRequest.types.ts (2)
1-2: Import is correctly defined.The
ApiResponsetype import is appropriately included.
17-17: Type alias is correctly defined.The
PretagResponsetype correctly extends theApiResponsetype with thePretagResponseDatainterface.app/client/src/git/components/ReleaseVersionRadioGroup/index.tsx (1)
4-4: Hook migration implementation is correct.The migration from
useLatestCommittousePretagis properly implemented with appropriate property access updates (releaseTagName, releasedAt). Proper optional chaining and nullish coalescing are used to handle potential undefined values.Also applies to: 13-13, 17-17, 19-19
app/client/src/git/store/helpers/initialState.ts (1)
132-136: New pretag state initialization looks good.The new
pretagproperty is correctly added to the state with the standard structure (value, loading, error) and appropriate initial values.app/client/src/git/components/OpsModal/TabRelease/TabReleaseView.tsx (4)
6-7: Props interface properly defined.The
TabReleasePropsinterface is well-defined with the appropriate function signature forfetchPretag.Also applies to: 25-27
29-29: Renamed component with proper prop destructuring.The component has been correctly renamed from
TabReleasetoTabReleaseViewwith default value handling using noop.
35-40: Appropriate effect hook implementation.The useEffect is correctly implemented with a descriptive name and proper dependency array to ensure fetchPretag is called on component mount and when the function reference changes.
72-72: Export statement updated correctly.Export statement properly reflects the new component name.
app/client/src/git/requests/pretagRequest.ts (1)
4-4:❓ Verification inconclusive
API request function renamed but endpoint unchanged.
The function has been appropriately renamed from
fetchLatestCommitRequesttopretagRequestwith updated return type. Note that the API endpoint still contains "commit/latest" in the URL - ensure this is aligned with backend expectations.Also applies to: 8-11
🏁 Script executed:
#!/bin/bash # Check if there are any references to the endpoint in other files that might need updating rg -A 2 "commit/latest" --glob "*.{ts,tsx}" | grep -v "pretagRequest.ts"Length of output: 70
Endpoint Verification: Please confirm backend alignment
The function has been successfully renamed to
pretagRequestwith the updated return type, but the API URL still includes"commit/latest"(notably around lines 8–11 ofapp/client/src/git/requests/pretagRequest.ts). Our initial search didn’t reveal other usages of this endpoint, so please manually verify that the backend still expects"commit/latest"for this API call.
- File:
app/client/src/git/requests/pretagRequest.ts(especially lines 8–11)- Action: Confirm with backend or update the endpoint as needed to match backend expectations
app/client/src/git/store/selectors/gitArtifactSelectors.ts (1)
91-94: Selector implementation looks good.New selector correctly retrieves the pretag state from API responses.
app/client/src/git/store/types.ts (2)
22-22: Import statement correctly added.The new import for PretagResponseData replaces the removed FetchLatestCommitResponseData import.
63-63: Type definition properly updated.The new pretag property replaces the removed latestCommit property in GitArtifactAPIResponsesReduxState.
app/client/src/git/store/gitArtifactSlice.ts (2)
145-148: Imports correctly updated.New pretag action imports replace the removed latestCommit actions.
213-215: Reducers properly defined.New pretag reducers added to replace the removed latestCommit reducers.
app/client/src/git/store/actions/pretagActions.ts (2)
20-27: Success action implementation looks good.The success action correctly updates the state with the pretag response data.
29-38: Error action implementation is correct.The error handling is properly implemented to update the state with error information.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/git/hooks/useReleaseTag.ts (1)
8-36: Well-structured hook implementationThe hook follows React best practices by using useCallback for memoization and properly setting up dependencies. It handles the release tag creation workflow cleanly.
Consider adding explicit TypeScript return type annotation for this hook to improve type safety and developer experience:
-function useReleaseTag() { +function useReleaseTag(): { + isCreateReleaseTagLoading: boolean; + createReleaseTagError: Error | null; + createReleaseTag: (params: { tag: string; releaseNote: string; commitSHA: string }) => void; +} {app/client/src/git/components/OpsModal/OpsModalView.tsx (1)
63-78: Consider adding analytics logging for the Release tab.The
handleTabKeyChangefunction logs analytics events for Deploy and Merge tabs, but there's no equivalent logging for the new Release tab. For consistency, consider adding analytics for the Release tab.if (tabKey === GitOpsTab.Deploy) { AnalyticsUtil.logEvent("GS_DEPLOY_GIT_MODAL_TRIGGERED", { source: `${tabKey.toUpperCase()}_TAB`, }); } else if (tabKey === GitOpsTab.Merge) { AnalyticsUtil.logEvent("GS_MERGE_GIT_MODAL_TRIGGERED", { source: `${tabKey.toUpperCase()}_TAB`, }); +} else if (tabKey === GitOpsTab.Release) { + AnalyticsUtil.logEvent("GS_RELEASE_GIT_MODAL_TRIGGERED", { + source: `${tabKey.toUpperCase()}_TAB`, + }); }app/client/src/git/components/LatestCommitInfo/LatestCommitInfoView.tsx (1)
28-31: Improve conditional rendering logic for better readability.The current implementation uses multiple conditional renders which makes the code harder to read. Consider using a more concise approach:
- {authorName && !committedAt ? `Committed by ${authorName}` : null} - {authorName && committedAt - ? `${authorName} committed ${committedAt ?? "-"}` - : null} + {authorName ? ( + committedAt + ? `${authorName} committed ${committedAt}` + : `Committed by ${authorName}` + ) : null}This approach consolidates the logic and makes it more maintainable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (18)
app/client/src/git/ce/constants/messages.tsx(1 hunks)app/client/src/git/components/LatestCommitInfo/LatestCommitInfoView.tsx(1 hunks)app/client/src/git/components/LatestCommitInfo/index.tsx(1 hunks)app/client/src/git/components/OpsModal/OpsModalView.tsx(4 hunks)app/client/src/git/components/OpsModal/TabRelease/TabReleaseView.tsx(4 hunks)app/client/src/git/components/OpsModal/TabRelease/index.tsx(1 hunks)app/client/src/git/components/OpsModal/index.tsx(2 hunks)app/client/src/git/components/ReleaseVersionRadioGroup/ReleaseVersionRadioGroupView.tsx(2 hunks)app/client/src/git/components/ReleaseVersionRadioGroup/index.tsx(2 hunks)app/client/src/git/helpers/isGitTaggingEnabled.ts(1 hunks)app/client/src/git/hooks/useReleaseTag.ts(1 hunks)app/client/src/git/requests/pretagRequest.ts(1 hunks)app/client/src/git/sagas/index.ts(2 hunks)app/client/src/git/sagas/pretagSaga.ts(1 hunks)app/client/src/git/store/actions/createReleaseTagActions.ts(1 hunks)app/client/src/git/store/gitArtifactSlice.ts(2 hunks)app/client/src/git/store/selectors/gitArtifactSelectors.ts(1 hunks)app/client/src/git/store/types.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/git/ce/constants/messages.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- app/client/src/git/components/LatestCommitInfo/index.tsx
- app/client/src/git/components/OpsModal/TabRelease/index.tsx
- app/client/src/git/store/types.ts
- app/client/src/git/components/ReleaseVersionRadioGroup/index.tsx
- app/client/src/git/store/selectors/gitArtifactSelectors.ts
- app/client/src/git/store/gitArtifactSlice.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (27)
app/client/src/git/helpers/isGitTaggingEnabled.ts (1)
4-10: Clean and straightforward implementationThe function correctly checks if Git tagging is enabled based on the artifact type.
The function could be simplified to a one-liner, but the current implementation with explicit control flow is clear and maintainable.
app/client/src/git/sagas/index.ts (2)
44-44: Correctly added new saga importThe pretagSaga import follows the existing pattern in the file.
90-92: Proper integration of pretagSagaThe pretag functionality is correctly integrated into the blocking action sagas, maintaining consistency with the application's saga architecture.
app/client/src/git/components/OpsModal/index.tsx (3)
8-8: Added GitContext integrationAppropriate import for accessing the Git context.
11-11: Correctly extracts artifactDef from contextThe component now uses the Git context to access the artifact definition.
22-22: Properly passes artifactDef to child componentThe artifactDef is correctly passed down to OpsModalView, enabling conditional rendering of the tagging functionality.
app/client/src/git/components/OpsModal/OpsModalView.tsx (3)
52-53: LGTM! Clean implementation of the conditional logic.The implementation correctly checks if tagging is enabled based on the provided artifact definition.
102-110: LGTM! Well-structured conditional tab rendering.The conditional rendering of the Release tab is properly implemented with appropriate data-testid attributes.
115-117: LGTM! Proper conditional component rendering.The TabRelease component is correctly rendered only when tagging is enabled and the corresponding tab is selected.
app/client/src/git/requests/pretagRequest.ts (3)
4-4: LGTM! Correct type import.The import statement correctly references the new
PretagResponsetype.
8-11: LGTM! Function signature properly updated.The function has been appropriately renamed from
fetchLatestCommitRequesttopretagRequestwith the correct return type.
13-13: LGTM! API endpoint updated correctly.The API endpoint has been updated from "commit/latest" to "pretag" to match the new functionality.
app/client/src/git/components/LatestCommitInfo/LatestCommitInfoView.tsx (1)
26-26: LGTM! Good use of nullish coalescing.The nullish coalescing operator provides a sensible default when
messageis null.app/client/src/git/sagas/pretagSaga.ts (2)
1-9: LGTM! Well-organized imports.The imports are logically organized and include all necessary dependencies for the saga.
10-36: LGTM! Well-structured saga implementation.The saga follows the standard pattern:
- Extracts data from the action payload
- Makes the API call
- Validates the response
- Dispatches success or error actions accordingly
- Includes proper error handling
The structure is clear, and the error handling is comprehensive.
app/client/src/git/components/OpsModal/TabRelease/TabReleaseView.tsx (5)
25-34: Well-defined interface with clear property typesThe TabReleaseProps interface clearly defines the contract for this component with appropriate types for each property.
36-41: Good use of default values for propsUsing default values like
noopfor functions andfalse/nullfor primitive values prevents runtime errors when props are not provided.
47-52: Proper effect implementation with named function and dependency arrayThe useEffect hook follows best practices by using a named function and correctly listing dependencies.
54-60: Callback handles null values correctlyGood use of nullish coalescing operators to handle potential null values safely.
80-80: Loading state implementation in UIProperly connecting the loading state to the button's isLoading prop improves user feedback.
app/client/src/git/components/ReleaseVersionRadioGroup/ReleaseVersionRadioGroupView.tsx (3)
10-10: Improved naming for better semantic clarityChanging from
currentVersiontolatestReleaseVersionprovides better semantic clarity about the purpose of this prop.
23-26: Simplified version calculation logicThe version calculation logic has been streamlined to only check for
releaseType, with proper fallback to "0.0.0" whenlatestReleaseVersionis null. Dependencies are correctly updated in the dependency array.
65-74: Improved conditional renderingThe conditional rendering now checks for
latestReleaseVersionand displays version information with appropriate formatting.app/client/src/git/store/actions/createReleaseTagActions.ts (4)
4-9: Well-structured payload interfaceThe CreateReleaseTagInitPayload interface properly defines all the required properties with clear names.
11-17: Clean implementation of initialization actionThe initialization action correctly sets loading state and clears previous errors.
19-23: Concise success actionThe success action does exactly what's needed - sets loading to false.
25-33: Complete error handlingThe error action properly extracts the error from the payload and updates the state accordingly.
|
|
||
| const handleClickOnRelease = useCallback(() => { | ||
| createReleaseTag({ | ||
| tag: releaseVersion ?? "", |
There was a problem hiding this comment.
Should we check for the presence of releaseVersion and then only dispatch this action?
Description
Fixes #38808
Automation
/ok-to-test tags="@tag.Git"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13706138270
Commit: 08cad2f
Cypress dashboard.
Tags:
@tag.GitSpec:
Thu, 06 Mar 2025 19:58:32 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
TabReleasecomponent for managing pre-tagging functionality.usePretagfor improved data management related to pre-tagging.useReleaseTaghook for handling release tag creation.OpsModalViewto conditionally render tagging capabilities based on artifact definition.isGitTaggingEnabledfunction to determine if tagging is applicable based on artifact type.Bug Fixes
LatestCommitInfoViewfor commit messages and author information.Refactor
Style
TAB_RELEASEproperty for improved display consistency.