-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[WEB-2388] fix: workspace draft issues #5800
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
ab84e5d
f170f1c
615df98
66b1e2e
b5a5f1f
4bb555b
96bd3d7
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 | ||||
|---|---|---|---|---|---|---|
| @@ -1,14 +1,17 @@ | ||||||
| "use client"; | ||||||
|
|
||||||
| import { FC } from "react"; | ||||||
| import { FC, Fragment } from "react"; | ||||||
| import { observer } from "mobx-react"; | ||||||
| import useSWR from "swr"; | ||||||
| // components | ||||||
| import { EmptyState } from "@/components/empty-state"; | ||||||
| // constants | ||||||
| import { EmptyStateType } from "@/constants/empty-state"; | ||||||
| import { EDraftIssuePaginationType } from "@/constants/workspace-drafts"; | ||||||
| // helpers | ||||||
| import { cn } from "@/helpers/common.helper"; | ||||||
| // hooks | ||||||
| import { useWorkspaceDraftIssues } from "@/hooks/store"; | ||||||
| import { useCommandPalette, useProject, useWorkspaceDraftIssues } from "@/hooks/store"; | ||||||
| // components | ||||||
| import { DraftIssueBlock } from "./draft-issue-block"; | ||||||
| import { WorkspaceDraftEmptyState } from "./empty-state"; | ||||||
|
|
@@ -21,7 +24,9 @@ type TWorkspaceDraftIssuesRoot = { | |||||
| export const WorkspaceDraftIssuesRoot: FC<TWorkspaceDraftIssuesRoot> = observer((props) => { | ||||||
| const { workspaceSlug } = props; | ||||||
| // hooks | ||||||
| const { loader, paginationInfo, fetchIssues, issuesMap, issueIds } = useWorkspaceDraftIssues(); | ||||||
| const { loader, paginationInfo, fetchIssues, issueIds } = useWorkspaceDraftIssues(); | ||||||
| const { workspaceProjectIds } = useProject(); | ||||||
| const { toggleCreateProjectModal } = useCommandPalette(); | ||||||
|
|
||||||
| // fetching issues | ||||||
| useSWR( | ||||||
|
|
@@ -39,6 +44,17 @@ export const WorkspaceDraftIssuesRoot: FC<TWorkspaceDraftIssuesRoot> = observer( | |||||
| return <WorkspaceDraftIssuesLoader items={14} />; | ||||||
| } | ||||||
|
|
||||||
| if (workspaceProjectIds?.length === 0) | ||||||
| return ( | ||||||
| <EmptyState | ||||||
| type={EmptyStateType.WORKSPACE_NO_PROJECTS} | ||||||
| size="sm" | ||||||
| primaryButtonOnClick={() => { | ||||||
| toggleCreateProjectModal(true); | ||||||
| }} | ||||||
| /> | ||||||
| ); | ||||||
|
|
||||||
| if (loader === "empty-state" && issueIds.length <= 0) return <WorkspaceDraftEmptyState />; | ||||||
|
|
||||||
| return ( | ||||||
|
|
@@ -48,22 +64,26 @@ export const WorkspaceDraftIssuesRoot: FC<TWorkspaceDraftIssuesRoot> = observer( | |||||
| <DraftIssueBlock key={issueId} workspaceSlug={workspaceSlug} issueId={issueId} /> | ||||||
| ))} | ||||||
| </div> | ||||||
| {loader === "pagination" && issueIds.length >= 0 ? ( | ||||||
| <WorkspaceDraftIssuesLoader items={1} /> | ||||||
| ) : ( | ||||||
| <div | ||||||
| className={cn( | ||||||
| "h-11 pl-6 p-3 text-sm font-medium bg-custom-background-100 border-b border-custom-border-200 transition-all", | ||||||
| { | ||||||
| "text-custom-primary-100 hover:text-custom-primary-200 cursor-pointer underline-offset-2 hover:underline": | ||||||
| paginationInfo?.next_page_results, | ||||||
| "text-custom-text-300 cursor-not-allowed": !paginationInfo?.next_page_results, | ||||||
| } | ||||||
|
|
||||||
| {paginationInfo?.next_page_results && ( | ||||||
| <Fragment> | ||||||
| {loader === "pagination" && issueIds.length >= 0 ? ( | ||||||
|
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. 🛠️ Refactor suggestion Simplify conditional rendering by removing redundant check The condition Apply this diff to simplify the condition: - {loader === "pagination" && issueIds.length >= 0 ? (
+ {loader === "pagination" ? (📝 Committable suggestion
Suggested change
|
||||||
| <WorkspaceDraftIssuesLoader items={1} /> | ||||||
| ) : ( | ||||||
| <div | ||||||
| className={cn( | ||||||
| "h-11 pl-6 p-3 text-sm font-medium bg-custom-background-100 border-b border-custom-border-200 transition-all", | ||||||
| { | ||||||
| "text-custom-primary-100 hover:text-custom-primary-200 cursor-pointer underline-offset-2 hover:underline": | ||||||
| paginationInfo?.next_page_results, | ||||||
| } | ||||||
| )} | ||||||
| onClick={handleNextIssues} | ||||||
| > | ||||||
| Load More ↓ | ||||||
| </div> | ||||||
| )} | ||||||
| onClick={handleNextIssues} | ||||||
| > | ||||||
| Load More ↓ | ||||||
| </div> | ||||||
| </Fragment> | ||||||
|
Comment on lines
+69
to
+86
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. 🛠️ Refactor suggestion Consider removing unnecessary The Apply this diff to remove the {paginationInfo?.next_page_results && (
- <Fragment>
{loader === "pagination" ? (
<WorkspaceDraftIssuesLoader items={1} />
) : (
<div
className={cn(
"h-11 pl-6 p-3 text-sm font-medium bg-custom-background-100 border-b border-custom-border-200 transition-all",
{
"text-custom-primary-100 hover:text-custom-primary-200 cursor-pointer underline-offset-2 hover:underline":
paginationInfo?.next_page_results,
}
)}
onClick={handleNextIssues}
>
Load More ↓
</div>
)}
- </Fragment>
)}Since -import { FC, Fragment } from "react";
+import { FC } from "react";
|
||||||
| )} | ||||||
| </div> | ||||||
| ); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,14 +24,17 @@ import { EDraftIssuePaginationType } from "@/constants/workspace-drafts"; | |||||||||||||||||
| import { getCurrentDateTimeInISO, convertToISODateString } from "@/helpers/date-time.helper"; | ||||||||||||||||||
| // services | ||||||||||||||||||
| import workspaceDraftService from "@/services/issue/workspace_draft.service"; | ||||||||||||||||||
| // types | ||||||||||||||||||
| import { IIssueRootStore } from "../root.store"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export type TDraftIssuePaginationType = EDraftIssuePaginationType; | ||||||||||||||||||
|
|
||||||||||||||||||
| export interface IWorkspaceDraftIssues { | ||||||||||||||||||
| // observables | ||||||||||||||||||
| issuesMap: Record<string, TWorkspaceDraftIssue>; | ||||||||||||||||||
| paginationInfo: Omit<TWorkspaceDraftPaginationInfo<TWorkspaceDraftIssue>, "results"> | undefined; | ||||||||||||||||||
| loader: TWorkspaceDraftIssueLoader; | ||||||||||||||||||
| paginationInfo: Omit<TWorkspaceDraftPaginationInfo<TWorkspaceDraftIssue>, "results"> | undefined; | ||||||||||||||||||
| issuesMap: Record<string, TWorkspaceDraftIssue>; // issue_id -> issue; | ||||||||||||||||||
| issueMapIds: Record<string, string[]>; // workspace_id -> issue_ids; | ||||||||||||||||||
| // computed | ||||||||||||||||||
| issueIds: string[]; | ||||||||||||||||||
| // computed functions | ||||||||||||||||||
|
|
@@ -112,15 +115,17 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
| // local constants | ||||||||||||||||||
| paginatedCount = 50; | ||||||||||||||||||
| // observables | ||||||||||||||||||
| paginationInfo: Omit<TWorkspaceDraftPaginationInfo<TWorkspaceDraftIssue>, "results"> | undefined = undefined; | ||||||||||||||||||
| loader: TWorkspaceDraftIssueLoader = undefined; | ||||||||||||||||||
| paginationInfo: Omit<TWorkspaceDraftPaginationInfo<TWorkspaceDraftIssue>, "results"> | undefined = undefined; | ||||||||||||||||||
| issuesMap: Record<string, TWorkspaceDraftIssue> = {}; | ||||||||||||||||||
| issueMapIds: Record<string, string[]> = {}; | ||||||||||||||||||
|
|
||||||||||||||||||
| constructor() { | ||||||||||||||||||
| constructor(public issueStore: IIssueRootStore) { | ||||||||||||||||||
| makeObservable(this, { | ||||||||||||||||||
| paginationInfo: observable, | ||||||||||||||||||
| loader: observable.ref, | ||||||||||||||||||
| paginationInfo: observable, | ||||||||||||||||||
| issuesMap: observable, | ||||||||||||||||||
| issueMapIds: observable, | ||||||||||||||||||
| // computed | ||||||||||||||||||
| issueIds: computed, | ||||||||||||||||||
| // action | ||||||||||||||||||
|
|
@@ -136,10 +141,11 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
|
|
||||||||||||||||||
| // computed | ||||||||||||||||||
| get issueIds() { | ||||||||||||||||||
| if (Object.keys(this.issuesMap).length <= 0) return []; | ||||||||||||||||||
| return orderBy(Object.values(this.issuesMap), (issue) => convertToISODateString(issue["created_at"]), ["asc"]).map( | ||||||||||||||||||
| (issue) => issue?.id | ||||||||||||||||||
| ); | ||||||||||||||||||
| const workspaceSlug = this.issueStore.workspaceSlug; | ||||||||||||||||||
| if (!workspaceSlug) return []; | ||||||||||||||||||
| if (!this.issueMapIds[workspaceSlug]) return []; | ||||||||||||||||||
| const issueIds = this.issueMapIds[workspaceSlug]; | ||||||||||||||||||
| return orderBy(issueIds, (issueId) => convertToISODateString(this.issuesMap[issueId]?.created_at), ["desc"]); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // computed functions | ||||||||||||||||||
|
|
@@ -216,7 +222,10 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
| const { results, ...paginationInfo } = draftIssuesResponse; | ||||||||||||||||||
| runInAction(() => { | ||||||||||||||||||
| if (results && results.length > 0) { | ||||||||||||||||||
| this.addIssue(results as TWorkspaceDraftIssue[]); | ||||||||||||||||||
| // adding issueIds | ||||||||||||||||||
| const issueIds = results.map((issue) => issue.id); | ||||||||||||||||||
| this.addIssue(results); | ||||||||||||||||||
| update(this.issueMapIds, [workspaceSlug], (existingIssueIds = []) => [...issueIds, ...existingIssueIds]); | ||||||||||||||||||
| this.loader = undefined; | ||||||||||||||||||
| } else { | ||||||||||||||||||
| this.loader = "empty-state"; | ||||||||||||||||||
|
|
@@ -240,7 +249,10 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
|
|
||||||||||||||||||
| const response = await workspaceDraftService.createIssue(workspaceSlug, payload); | ||||||||||||||||||
| if (response) { | ||||||||||||||||||
| runInAction(() => set(this.issuesMap, response.id, response)); | ||||||||||||||||||
| runInAction(() => { | ||||||||||||||||||
| this.addIssue([response]); | ||||||||||||||||||
| update(this.issueMapIds, [workspaceSlug], (existingIssueIds = []) => [response.id, ...existingIssueIds]); | ||||||||||||||||||
| }); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| this.loader = undefined; | ||||||||||||||||||
|
|
@@ -256,8 +268,11 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
| try { | ||||||||||||||||||
| this.loader = "update"; | ||||||||||||||||||
| runInAction(() => { | ||||||||||||||||||
| set(this.issuesMap, [issueId, "updated_at"], getCurrentDateTimeInISO()); | ||||||||||||||||||
| set(this.issuesMap, [issueId], { ...issueBeforeUpdate, ...payload }); | ||||||||||||||||||
| set(this.issuesMap, [issueId], { | ||||||||||||||||||
| ...issueBeforeUpdate, | ||||||||||||||||||
| ...payload, | ||||||||||||||||||
| ...{ updated_at: getCurrentDateTimeInISO() }, | ||||||||||||||||||
| }); | ||||||||||||||||||
| }); | ||||||||||||||||||
| const response = await workspaceDraftService.updateIssue(workspaceSlug, issueId, payload); | ||||||||||||||||||
| this.loader = undefined; | ||||||||||||||||||
|
|
@@ -276,7 +291,10 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
| this.loader = "delete"; | ||||||||||||||||||
|
|
||||||||||||||||||
| const response = await workspaceDraftService.deleteIssue(workspaceSlug, issueId); | ||||||||||||||||||
| runInAction(() => unset(this.issuesMap, issueId)); | ||||||||||||||||||
| runInAction(() => { | ||||||||||||||||||
| unset(this.issueMapIds[workspaceSlug], issueId); | ||||||||||||||||||
| unset(this.issuesMap, issueId); | ||||||||||||||||||
| }); | ||||||||||||||||||
|
Comment on lines
+294
to
+297
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. Fix issue removal: incorrect use of The Suggested fix: runInAction(() => {
- unset(this.issueMapIds[workspaceSlug], issueId);
+ update(this.issueMapIds, [workspaceSlug], (issueIds = []) => issueIds.filter(id => id !== issueId));
unset(this.issuesMap, issueId);
});📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| this.loader = undefined; | ||||||||||||||||||
| return response; | ||||||||||||||||||
|
|
@@ -291,7 +309,10 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues { | |||||||||||||||||
| this.loader = "move"; | ||||||||||||||||||
|
|
||||||||||||||||||
| const response = await workspaceDraftService.moveIssue(workspaceSlug, issueId, payload); | ||||||||||||||||||
| runInAction(() => unset(this.issuesMap, issueId)); | ||||||||||||||||||
| runInAction(() => { | ||||||||||||||||||
| unset(this.issueMapIds[workspaceSlug], issueId); | ||||||||||||||||||
| unset(this.issuesMap, issueId); | ||||||||||||||||||
| }); | ||||||||||||||||||
|
Comment on lines
+312
to
+315
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. Fix issue removal in Similar to Suggested fix: runInAction(() => {
- unset(this.issueMapIds[workspaceSlug], issueId);
+ update(this.issueMapIds, [workspaceSlug], (issueIds = []) => issueIds.filter(id => id !== issueId));
unset(this.issuesMap, issueId);
});📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| this.loader = undefined; | ||||||||||||||||||
| return response; | ||||||||||||||||||
|
|
||||||||||||||||||
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.
Ensure proper handling when
workspaceProjectIdsis undefinedAt line 47, the condition
workspaceProjectIds?.length === 0checks for an empty array. However, ifworkspaceProjectIdsisundefined,workspaceProjectIds?.lengthwill beundefined, and the condition will evaluate tofalse. If you intend to show theEmptyStatewhenworkspaceProjectIdsisundefinedor empty, consider updating the condition.Apply this diff to handle both
undefinedand empty arrays:📝 Committable suggestion