-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[WEB-2479] fix: merge default and archived issue details endpoint. #5882
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,9 @@ import { ISSUE_DETAILS } from "@/constants/fetch-keys"; | |
| // hooks | ||
| import { useProject } from "@/hooks/store"; | ||
| // services | ||
| import { IssueArchiveService } from "@/services/issue"; | ||
| import { IssueService } from "@/services/issue"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Unresolved references to IssueArchiveService found. The following files still reference
Please update these to use 🔗 Analysis chainLGTM! Verify impact on other components. The change from To ensure this change doesn't introduce unintended side effects, please run the following script to check for any remaining references to Also applies to: 18-18 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any remaining references to IssueArchiveService
# Test: Search for IssueArchiveService usage
rg 'IssueArchiveService'
Length of output: 621 |
||
|
|
||
| const issueArchiveService = new IssueArchiveService(); | ||
| const issueService = new IssueService(); | ||
|
|
||
| export const ProjectArchivedIssueDetailsHeader = observer(() => { | ||
| // router | ||
|
|
@@ -24,14 +24,9 @@ export const ProjectArchivedIssueDetailsHeader = observer(() => { | |
| const { currentProjectDetails, loader } = useProject(); | ||
|
|
||
| const { data: issueDetails } = useSWR( | ||
| workspaceSlug && projectId && archivedIssueId ? ISSUE_DETAILS(archivedIssueId as string) : null, | ||
| workspaceSlug && projectId && archivedIssueId ? ISSUE_DETAILS(archivedIssueId.toString()) : null, | ||
| workspaceSlug && projectId && archivedIssueId | ||
| ? () => | ||
| issueArchiveService.retrieveArchivedIssue( | ||
| workspaceSlug as string, | ||
| projectId as string, | ||
| archivedIssueId as string | ||
| ) | ||
| ? () => issueService.retrieve(workspaceSlug.toString(), projectId.toString(), archivedIssueId.toString()) | ||
| : null | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ export const ArchivedIssueLayoutRoot: React.FC = observer(() => { | |
| <div className="relative h-full w-full overflow-auto"> | ||
| <ArchivedIssueListLayout /> | ||
| </div> | ||
| <IssuePeekOverview is_archived /> | ||
| <IssuePeekOverview /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Remaining usage of
🔗 Analysis chainVerify the removal of The However, to ensure this change doesn't introduce any unintended side effects:
To verify this change, please run the following script: This script will help us confirm that the 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any remaining uses of 'is_archived' prop in IssuePeekOverview
# Search for IssuePeekOverview component definition
echo "IssuePeekOverview component definition:"
ast-grep --pattern 'const IssuePeekOverview = $_' -A 10
# Search for uses of IssuePeekOverview component
echo "\nUses of IssuePeekOverview component:"
rg '<IssuePeekOverview' -A 3 -g '!archived-issue-layout-root.tsx'
# Check if 'is_archived' is still used in the component
echo "\nChecking for 'is_archived' usage in IssuePeekOverview:"
rg 'is_archived' $(rg -l 'const IssuePeekOverview = $_')
Length of output: 8618 |
||
| </Fragment> | ||
| </IssuesStoreContext.Provider> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| "use client"; | ||
|
|
||
| import { FC, useEffect, useState, useMemo } from "react"; | ||
| import { FC, useEffect, useState, useMemo, useCallback } from "react"; | ||
| import { observer } from "mobx-react"; | ||
| import { usePathname } from "next/navigation"; | ||
| // plane types | ||
|
|
@@ -21,12 +21,11 @@ import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/u | |
| interface IIssuePeekOverview { | ||
| embedIssue?: boolean; | ||
| embedRemoveCurrentNotification?: () => void; | ||
| is_archived?: boolean; | ||
| is_draft?: boolean; | ||
| } | ||
|
|
||
| export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | ||
| const { embedIssue = false, embedRemoveCurrentNotification, is_archived = false, is_draft = false } = props; | ||
| const { embedIssue = false, embedRemoveCurrentNotification, is_draft = false } = props; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification
The
🔗 Analysis chainVerify the removal of The removal of the To verify this change, run the following command to check for any remaining uses of 🏁 Scripts executedThe following scripts were executed for the analysis: Script: rg "is_archived" --type tsx --type ts
Length of output: 65 Script: #!/bin/bash
# Searching for 'is_archived' in .ts and .tsx files
rg "is_archived" --glob "*.ts" --glob "*.tsx"
Length of output: 2428 |
||
| // router | ||
| const pathname = usePathname(); | ||
| // store hook | ||
|
|
@@ -47,26 +46,20 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| // state | ||
| const [error, setError] = useState(false); | ||
|
|
||
| const removeRoutePeekId = () => { | ||
| const removeRoutePeekId = useCallback(() => { | ||
| setPeekIssue(undefined); | ||
| if (embedIssue) embedRemoveCurrentNotification?.(); | ||
| }; | ||
| }, [embedIssue, embedRemoveCurrentNotification, setPeekIssue]); | ||
|
|
||
| const issueOperations: TIssueOperations = useMemo( | ||
| () => ({ | ||
| fetch: async (workspaceSlug: string, projectId: string, issueId: string) => { | ||
| try { | ||
| setError(false); | ||
| await fetchIssue( | ||
| workspaceSlug, | ||
| projectId, | ||
| issueId, | ||
| is_archived ? "ARCHIVED" : is_draft ? "DRAFT" : "DEFAULT" | ||
| ); | ||
| setError(false); | ||
| await fetchIssue(workspaceSlug, projectId, issueId, is_draft ? "DRAFT" : "DEFAULT"); | ||
| } catch (error) { | ||
| setError(true); | ||
| console.error("Error fetching the parent issue"); | ||
| console.error("Error fetching the parent issue", error); | ||
| } | ||
| }, | ||
| update: async (workspaceSlug: string, projectId: string, issueId: string, data: Partial<TIssue>) => { | ||
|
|
@@ -109,7 +102,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }); | ||
| removeRoutePeekId(); | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| setToast({ | ||
| title: "Error!", | ||
| type: TOAST_TYPE.ERROR, | ||
|
|
@@ -124,13 +117,14 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }, | ||
| archive: async (workspaceSlug: string, projectId: string, issueId: string) => { | ||
| try { | ||
| issues?.archiveIssue && (await issues.archiveIssue(workspaceSlug, projectId, issueId)); | ||
| if (!issues?.archiveIssue) return; | ||
| await issues.archiveIssue(workspaceSlug, projectId, issueId); | ||
| captureIssueEvent({ | ||
| eventName: ISSUE_ARCHIVED, | ||
| payload: { id: issueId, state: "SUCCESS", element: "Issue peek-overview" }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| captureIssueEvent({ | ||
| eventName: ISSUE_ARCHIVED, | ||
| payload: { id: issueId, state: "FAILED", element: "Issue peek-overview" }, | ||
|
|
@@ -151,7 +145,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| payload: { id: issueId, state: "SUCCESS", element: "Issue peek-overview" }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| setToast({ | ||
| type: TOAST_TYPE.ERROR, | ||
| title: "Error!", | ||
|
|
@@ -177,7 +171,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| setToast({ | ||
| type: TOAST_TYPE.ERROR, | ||
| title: "Error!", | ||
|
|
@@ -206,7 +200,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| setToast({ | ||
| type: TOAST_TYPE.ERROR, | ||
| title: "Error!", | ||
|
|
@@ -248,7 +242,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| captureIssueEvent({ | ||
| eventName: ISSUE_UPDATED, | ||
| payload: { state: "FAILED", element: "Issue peek-overview" }, | ||
|
|
@@ -311,7 +305,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| }, | ||
| path: pathname, | ||
| }); | ||
| } catch (error) { | ||
| } catch { | ||
| captureIssueEvent({ | ||
| eventName: ISSUE_UPDATED, | ||
| payload: { id: issueId, state: "FAILED", element: "Issue peek-overview" }, | ||
|
|
@@ -324,7 +318,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| } | ||
| }, | ||
| }), | ||
| [is_archived, is_draft, fetchIssue, issues, restoreIssue, captureIssueEvent, pathname] | ||
| [fetchIssue, is_draft, issues, fetchActivities, captureIssueEvent, pathname, removeRoutePeekId, restoreIssue] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -350,7 +344,7 @@ export const IssuePeekOverview: FC<IIssuePeekOverview> = observer((props) => { | |
| issueId={peekIssue.issueId} | ||
| isLoading={getIsFetchingIssueDetails(peekIssue.issueId)} | ||
| isError={error} | ||
| is_archived={is_archived} | ||
| is_archived={!!peekIssue.isArchived} | ||
| disabled={!isEditable} | ||
| embedIssue={embedIssue} | ||
| embedRemoveCurrentNotification={embedRemoveCurrentNotification} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ export interface IIssueStoreActions { | |
| workspaceSlug: string, | ||
| projectId: string, | ||
| issueId: string, | ||
| issueType?: "DEFAULT" | "DRAFT" | "ARCHIVED" | ||
| issueStatus?: "DEFAULT" | "DRAFT", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Unresolved references to "ARCHIVED" found.
🔗 Analysis chainLGTM! Verify impact on codebase. The simplification of the Please run the following script to ensure that there are no remaining references to the "ARCHIVED" status in the codebase: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any remaining references to "ARCHIVED" status
rg '"ARCHIVED"' --type ts
Length of output: 4764 Script: #!/bin/bash
# Search for any remaining references to "ARCHIVED" status
rg '"ARCHIVED"' --type ts
Length of output: 80 |
||
| ) => Promise<TIssue>; | ||
| updateIssue: (workspaceSlug: string, projectId: string, issueId: string, data: Partial<TIssue>) => Promise<void>; | ||
| removeIssue: (workspaceSlug: string, projectId: string, issueId: string) => Promise<void>; | ||
|
|
@@ -82,7 +82,7 @@ export class IssueStore implements IIssueStore { | |
| }); | ||
|
|
||
| // actions | ||
| fetchIssue = async (workspaceSlug: string, projectId: string, issueId: string, issueType = "DEFAULT") => { | ||
| fetchIssue = async (workspaceSlug: string, projectId: string, issueId: string, issueStatus = "DEFAULT") => { | ||
| const query = { | ||
| expand: "issue_reactions,issue_attachments,issue_link,parent", | ||
| }; | ||
|
|
@@ -99,9 +99,7 @@ export class IssueStore implements IIssueStore { | |
| this.localDBIssueDescription = issueId; | ||
| } | ||
|
|
||
| if (issueType === "ARCHIVED") | ||
| issue = await this.issueArchiveService.retrieveArchivedIssue(workspaceSlug, projectId, issueId, query); | ||
| else if (issueType === "DRAFT") | ||
| if (issueStatus === "DRAFT") | ||
| issue = await this.issueDraftService.getDraftIssueById(workspaceSlug, projectId, issueId, query); | ||
| else issue = await this.issueService.retrieve(workspaceSlug, projectId, issueId, query); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ export type TPeekIssue = { | |
| projectId: string; | ||
| issueId: string; | ||
| nestingLevel?: number; | ||
| isArchived?: boolean; | ||
| }; | ||
|
|
||
| export type TIssueRelationModal = { | ||
|
|
@@ -251,8 +252,8 @@ export class IssueDetail implements IIssueDetail { | |
| workspaceSlug: string, | ||
| projectId: string, | ||
| issueId: string, | ||
| issueType: "DEFAULT" | "ARCHIVED" | "DRAFT" = "DEFAULT" | ||
| ) => this.issue.fetchIssue(workspaceSlug, projectId, issueId, issueType); | ||
| issueStatus: "DEFAULT" | "DRAFT" = "DEFAULT" | ||
| ) => this.issue.fetchIssue(workspaceSlug, projectId, issueId, issueStatus); | ||
|
Comment on lines
+255
to
+256
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Potential Issues: Incomplete The following calls to
Please update these calls to include the 🔗 Analysis chainLGTM: Updated The update to the To ensure this change doesn't break existing code, please verify all calls to 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Find all calls to fetchIssue method
# Expected result: All calls should use "DEFAULT" or "DRAFT" as the last argument, or omit it entirely
rg -p 'fetchIssue\s*\([^)]*\)' --type ts
Length of output: 857 |
||
| updateIssue = async (workspaceSlug: string, projectId: string, issueId: string, data: Partial<TIssue>) => | ||
| this.issue.updateIssue(workspaceSlug, projectId, issueId, data); | ||
| removeIssue = async (workspaceSlug: string, projectId: string, issueId: string) => | ||
|
|
||
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.
🛠️ Refactor suggestion
Refactor repeated queryset logic into a helper method
The queryset annotations and prefetches in the
retrievemethod are similar to those inpartial_update. To reduce code duplication and enhance maintainability, consider refactoring this logic into a shared helper method or modifyingget_queryset()to include the necessary annotations.Use
self.get_queryset()to ensure consistent queryingIn the
retrievemethod, you're directly queryingIssue.objectsinstead of usingself.get_queryset(). This may bypass filters, annotations, or prefetches defined inget_queryset(), potentially leading to inconsistencies. It's recommended to useself.get_queryset().filter(pk=pk)to maintain consistency across the viewset methods.Apply this diff to fix the issue:
📝 Committable suggestion