Skip to content
2 changes: 2 additions & 0 deletions apiserver/plane/app/serializers/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ class IssueCreateSerializer(BaseSerializer):
write_only=True,
required=False,
)
project_id = serializers.UUIDField(source="project.id", read_only=True)
workspace_id = serializers.UUIDField(source="workspace.id", read_only=True)

class Meta:
model = Issue
Expand Down
8 changes: 7 additions & 1 deletion web/core/local-db/storage.sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { rootStore } from "@/lib/store-context";
// services
import { IssueService } from "@/services/issue/issue.service";
//
import { ARRAY_FIELDS } from "./utils/constants";
import { ARRAY_FIELDS, BOOLEAN_FIELDS } from "./utils/constants";
import { getSubIssuesWithDistribution } from "./utils/data.utils";
import createIndexes from "./utils/indexes";
import { addIssuesBulk, syncDeletesToLocal } from "./utils/load-issues";
Expand Down Expand Up @@ -75,6 +75,7 @@ export class Storage {
if (workspaceSlug !== this.workspaceSlug) {
this.reset();
}

try {
await startSpan({ name: "INIT_DB" }, async () => await this._initialize(workspaceSlug));
return true;
Expand Down Expand Up @@ -125,6 +126,7 @@ export class Storage {
return true;
} catch (error) {
this.status = "error";
this.db = null;
throw new Error(`Failed to initialize database worker: ${error}`);
}
};
Expand Down Expand Up @@ -467,5 +469,9 @@ export const formatLocalIssue = (issue: any) => {
ARRAY_FIELDS.forEach((field: string) => {
currIssue[field] = currIssue[field] ? JSON.parse(currIssue[field]) : [];
});
// Convert boolean fields to actual boolean values
BOOLEAN_FIELDS.forEach((field: string) => {
currIssue[field] = currIssue[field] === 1;
});
return currIssue as TIssue & { group_id?: string; total_issues: number; sub_group_id?: string };
};
2 changes: 2 additions & 0 deletions web/core/local-db/utils/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export const ARRAY_FIELDS = ["label_ids", "assignee_ids", "module_ids"];

export const BOOLEAN_FIELDS = ["is_draft"];

Comment on lines +3 to +4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the search results, particularly from web/core/local-db/utils/schemas.ts and other files, I can now generate a final response.

Update BOOLEAN_FIELDS constant to include all relevant boolean fields

The constant should include additional boolean fields found in the codebase:

  • is_draft
  • is_local_update
  • is_bot
  • is_favorite
  • is_locked
  • is_subscribed

These fields are used across various models and schemas in the application, particularly in issues and pages.

🔗 Analysis chain

Verify completeness of boolean fields list.

Let's ensure we haven't missed any boolean fields that should be included in this constant.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for potential boolean fields in the codebase
# Look for field definitions or type annotations containing 'boolean' or 'bool'
# focusing on model/schema definitions and database-related files

echo "Searching for potential boolean fields in models and schemas..."
rg -i "boolean|bool" --type ts --type tsx web/

Length of output: 213


Script:

#!/bin/bash
# Let's try a different approach to find boolean fields
# 1. Search for boolean type annotations in TypeScript files
# 2. Search for boolean fields in schema definitions
# 3. Search for is_ prefixed fields which are commonly boolean

echo "Searching for boolean type annotations..."
rg -t ts ":\s*(boolean|Boolean)" web/

echo -e "\nSearching for is_ prefixed fields..."
rg -t ts "is_\w+\s*[:=]" web/

echo -e "\nSearching specifically in schema/model files..."
fd "schema|model" web/ -e ts -x rg -l "boolean|Boolean|is_\w+"

Length of output: 105064

export const GROUP_BY_MAP = {
state_id: "state_id",
priority: "priority",
Expand Down
71 changes: 39 additions & 32 deletions web/core/local-db/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,44 @@ export const logError = (e: any) => {
};
export const logInfo = console.info;

export const addIssueToPersistanceLayer = async (issue: TIssue) => {
try {
const issuePartial = pick({ ...JSON.parse(JSON.stringify(issue)) }, [
"id",
"name",
"state_id",
"sort_order",
"completed_at",
"estimate_point",
"priority",
"start_date",
"target_date",
"sequence_id",
"project_id",
"parent_id",
"created_at",
"updated_at",
"created_by",
"updated_by",
"is_draft",
"archived_at",
"state__group",
"cycle_id",
"link_count",
"attachment_count",
"sub_issues_count",
"assignee_ids",
"label_ids",
"module_ids",
"type_id",
"description_html",
]);
await updateIssue({ ...issuePartial, is_local_update: 1 });
} catch (e) {
logError("Error while adding issue to db");
}
};
Comment on lines +22 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling and code structure in addIssueToPersistanceLayer.

Several improvements can be made to enhance reliability and maintainability:

  1. The error message should include more details about the failure
  2. The JSON serialization/deserialization is unnecessary and could impact performance
  3. The list of picked fields should be extracted to a constant
  4. Consider adding return value for error handling

Here's the suggested implementation:

+const ISSUE_PERSISTENCE_FIELDS = [
+  "id", "name", "state_id", "sort_order", "completed_at", "estimate_point",
+  "priority", "start_date", "target_date", "sequence_id", "project_id",
+  "parent_id", "created_at", "updated_at", "created_by", "updated_by",
+  "is_draft", "archived_at", "state__group", "cycle_id", "link_count",
+  "attachment_count", "sub_issues_count", "assignee_ids", "label_ids",
+  "module_ids", "type_id", "description_html"
+] as const;

-export const addIssueToPersistanceLayer = async (issue: TIssue) => {
+export const addIssueToPersistanceLayer = async (issue: TIssue): Promise<boolean> => {
   try {
-    const issuePartial = pick({ ...JSON.parse(JSON.stringify(issue)) }, [
-      "id", "name", "state_id", /* ... other fields ... */
-    ]);
+    const issuePartial = pick(issue, ISSUE_PERSISTENCE_FIELDS);
     await updateIssue({ ...issuePartial, is_local_update: 1 });
+    return true;
   } catch (e) {
-    logError("Error while adding issue to db");
+    logError(`Failed to add issue ${issue.id} to db: ${e.message}`);
+    return false;
   }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const addIssueToPersistanceLayer = async (issue: TIssue) => {
try {
const issuePartial = pick({ ...JSON.parse(JSON.stringify(issue)) }, [
"id",
"name",
"state_id",
"sort_order",
"completed_at",
"estimate_point",
"priority",
"start_date",
"target_date",
"sequence_id",
"project_id",
"parent_id",
"created_at",
"updated_at",
"created_by",
"updated_by",
"is_draft",
"archived_at",
"state__group",
"cycle_id",
"link_count",
"attachment_count",
"sub_issues_count",
"assignee_ids",
"label_ids",
"module_ids",
"type_id",
"description_html",
]);
await updateIssue({ ...issuePartial, is_local_update: 1 });
} catch (e) {
logError("Error while adding issue to db");
}
};
const ISSUE_PERSISTENCE_FIELDS = [
"id", "name", "state_id", "sort_order", "completed_at", "estimate_point",
"priority", "start_date", "target_date", "sequence_id", "project_id",
"parent_id", "created_at", "updated_at", "created_by", "updated_by",
"is_draft", "archived_at", "state__group", "cycle_id", "link_count",
"attachment_count", "sub_issues_count", "assignee_ids", "label_ids",
"module_ids", "type_id", "description_html"
] as const;
export const addIssueToPersistanceLayer = async (issue: TIssue): Promise<boolean> => {
try {
const issuePartial = pick(issue, ISSUE_PERSISTENCE_FIELDS);
await updateIssue({ ...issuePartial, is_local_update: 1 });
return true;
} catch (e) {
logError(`Failed to add issue ${issue.id} to db: ${e.message}`);
return false;
}
};


export const updatePersistentLayer = async (issueIds: string | string[]) => {
if (typeof issueIds === "string") {
issueIds = [issueIds];
Expand All @@ -28,38 +66,7 @@ export const updatePersistentLayer = async (issueIds: string | string[]) => {
const issue = rootStore.issue.issues.getIssueById(issueId);

if (issue) {
// JSON.parse(JSON.stringify(issue)) is used to remove the mobx observables
const issuePartial = pick({ ...dbIssue, ...JSON.parse(JSON.stringify(issue)) }, [
"id",
"name",
"state_id",
"sort_order",
"completed_at",
"estimate_point",
"priority",
"start_date",
"target_date",
"sequence_id",
"project_id",
"parent_id",
"created_at",
"updated_at",
"created_by",
"updated_by",
"is_draft",
"archived_at",
"state__group",
"cycle_id",
"link_count",
"attachment_count",
"sub_issues_count",
"assignee_ids",
"label_ids",
"module_ids",
"type_id",
"description_html",
]);
await updateIssue({ ...issuePartial, is_local_update: 1 });
addIssueToPersistanceLayer(issue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix async operation handling in updatePersistentLayer.

The current implementation has potential issues:

  1. Using forEach with async operations doesn't wait for completion
  2. Missing error handling for addIssueToPersistanceLayer
  3. Silent failure when issue is not found

Here's the suggested fix:

 export const updatePersistentLayer = async (issueIds: string | string[]) => {
   if (typeof issueIds === "string") {
     issueIds = [issueIds];
   }
-  issueIds.forEach(async (issueId) => {
+  await Promise.all(issueIds.map(async (issueId) => {
     const dbIssue = await persistence.getIssue(issueId);
     const issue = rootStore.issue.issues.getIssueById(issueId);
 
-    if (issue) {
-      addIssueToPersistanceLayer(issue);
+    if (!issue) {
+      logError(`Issue ${issueId} not found in store`);
+      return;
     }
-  });
+    
+    const success = await addIssueToPersistanceLayer(issue);
+    if (!success) {
+      logError(`Failed to update issue ${issueId} in persistent layer`);
+    }
+  }));
 };

Committable suggestion skipped: line range outside the PR's diff.

}
});
};
Expand Down
16 changes: 12 additions & 4 deletions web/core/local-db/worker/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,20 @@ export class DBClass {
this.sqlite3 = SQLite.Factory(m);
const vfs = await MyVFS.create("plane", m);
this.sqlite3.vfs_register(vfs, true);
const db = await this.sqlite3.open_v2(
// Fallback in rare cases where the database is not initialized in time
const p = new Promise((resolve) => setTimeout(() => resolve(false), 2000));
const dbPromise = this.sqlite3.open_v2(
`${dbName}.sqlite3`,
this.sqlite3.OPEN_READWRITE | this.sqlite3.OPEN_CREATE,
"plane"
);

const db = await Promise.any([dbPromise, p]);

if (!db) {
throw new Error("Failed to initialize in time");
}

this.instance.db = db;
this.instance.exec = async (sql: string) => {
const rows: any[] = [];
Expand All @@ -58,7 +66,7 @@ export class DBClass {
}

runQuery(sql: string) {
return this.instance.exec(sql);
return this.instance?.exec?.(sql);
}

async exec(props: string | TQueryProps) {
Expand Down Expand Up @@ -103,14 +111,14 @@ export class DBClass {
}

if (sql === "COMMIT;" && this.tp) {
await this.instance.exec(sql);
await this.instance?.exec?.(sql);
if (this.tp.length > 0) {
const { resolve } = this.tpResolver.shift();
resolve();
}
return;
}
return await this.instance.exec(sql);
return await this.instance?.exec?.(sql);
}
async close() {
try {
Expand Down
2 changes: 1 addition & 1 deletion web/core/services/issue/workspace_draft.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class WorkspaceDraftService extends APIService {
});
}

async moveIssue(workspaceSlug: string, issueId: string, payload: Partial<TWorkspaceDraftIssue>): Promise<void> {
async moveIssue(workspaceSlug: string, issueId: string, payload: Partial<TWorkspaceDraftIssue>): Promise<TIssue> {
return this.post(`/api/workspaces/${workspaceSlug}/draft-to-issue/${issueId}/`, payload)
.then((response) => response?.data)
.catch((error) => {
Expand Down
8 changes: 8 additions & 0 deletions web/core/store/inbox/inbox-issue.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { makeObservable, observable, runInAction, action } from "mobx";
import { TIssue, TInboxIssue, TInboxIssueStatus, TInboxDuplicateIssueDetails } from "@plane/types";
// helpers
import { EInboxIssueStatus } from "@/helpers/inbox.helper";
// local db
import { addIssueToPersistanceLayer } from "@/local-db/utils/utils";
// services
import { InboxIssueService } from "@/services/inbox";
import { IssueService } from "@/services/issue";
Expand Down Expand Up @@ -88,10 +90,16 @@ export class InboxIssueStore implements IInboxIssueStore {

try {
if (!this.issue.id) return;

const inboxIssue = await this.inboxIssueService.update(this.workspaceSlug, this.projectId, this.issue.id, {
status: status,
});
runInAction(() => set(this, "status", inboxIssue?.status));

// If issue accepted sync issue to local db
if (status === EInboxIssueStatus.ACCEPTED) {
addIssueToPersistanceLayer(inboxIssue.issue);
}
Comment on lines +99 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance error handling for local DB synchronization.

The local DB sync operation is not wrapped in error handling, which could lead to silent failures. Consider:

  1. Wrapping the addIssueToPersistanceLayer call in a try-catch
  2. Adding a fallback mechanism if the sync fails
  3. Implementing a timeout for the sync operation

Here's a suggested improvement:

   // If issue accepted sync issue to local db
   if (status === EInboxIssueStatus.ACCEPTED) {
-    addIssueToPersistanceLayer(inboxIssue.issue);
+    try {
+      await Promise.race([
+        addIssueToPersistanceLayer(inboxIssue.issue),
+        new Promise((_, reject) => 
+          setTimeout(() => reject(new Error('DB sync timeout')), 5000)
+        )
+      ]);
+    } catch (error) {
+      console.error('Failed to sync issue to local DB:', error);
+      // Implement fallback mechanism here
+    }
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// If issue accepted sync issue to local db
if (status === EInboxIssueStatus.ACCEPTED) {
addIssueToPersistanceLayer(inboxIssue.issue);
}
// If issue accepted sync issue to local db
if (status === EInboxIssueStatus.ACCEPTED) {
try {
await Promise.race([
addIssueToPersistanceLayer(inboxIssue.issue),
new Promise((_, reject) =>
setTimeout(() => reject(new Error('DB sync timeout')), 5000)
)
]);
} catch (error) {
console.error('Failed to sync issue to local DB:', error);
// Implement fallback mechanism here
}
}

💡 Codebase verification

Based on the implementation found, I can now provide the final response:

DB error handling needs improvement

The current implementation of addIssueToPersistanceLayer has basic error handling but could be enhanced:

  • It catches errors but only logs them using logError("Error while adding issue to db")
  • No error propagation to the caller, making it impossible to handle DB failures at the inbox level
  • The error message is generic and doesn't include the actual error details

Suggested improvements:

  • Pass the actual error object to logError instead of just a string message
  • Consider propagating the error to allow proper handling in the inbox store
  • Add retry mechanism for transient DB failures
🔗 Analysis chain

Verify the local DB sync implementation.

Let's verify the implementation of addIssueToPersistanceLayer to ensure it properly handles DB initialization failures.


Let me gather more information about the error handling in the implementation.


Let me try one more search to find the complete implementation and error handling:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the implementation and error handling in addIssueToPersistanceLayer
echo "Searching for addIssueToPersistanceLayer implementation..."
rg -A 10 "export.*addIssueToPersistanceLayer"

# Search for DB initialization failure handling
echo "Searching for DB initialization handling..."
rg -l "db.*initialize.*catch|db.*init.*error"

Length of output: 1069


Script:

#!/bin/bash
# Get full implementation of addIssueToPersistanceLayer including error handling
rg -A 20 "export const addIssueToPersistanceLayer" web/core/local-db/utils/utils.ts

# Search for any DB-related error handling patterns
rg "catch.*db|db.*error" web/core/local-db/utils/utils.ts

# Look for where the DB is initialized
rg -A 10 "initializeDb|setupDb|createDb" web/core/local-db/

Length of output: 706


Script:

#!/bin/bash
# Search for the complete file content to see the full implementation
cat web/core/local-db/utils/utils.ts

# Also search for any error handling patterns in the file
rg "catch|throw|error" web/core/local-db/utils/utils.ts

# Look for DB initialization in index files
rg -A 10 "db" web/core/local-db/index.ts

Length of output: 5661

} catch {
runInAction(() => set(this, "status", previousData.status));
}
Expand Down
8 changes: 7 additions & 1 deletion web/core/store/issue/workspace-draft/issue.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
import { EDraftIssuePaginationType } from "@/constants/workspace-drafts";
// helpers
import { getCurrentDateTimeInISO, convertToISODateString } from "@/helpers/date-time.helper";
// local-db
import { addIssueToPersistanceLayer } from "@/local-db/utils/utils";
// services
import workspaceDraftService from "@/services/issue/workspace_draft.service";
// types
Expand Down Expand Up @@ -59,7 +61,7 @@ export interface IWorkspaceDraftIssues {
payload: Partial<TWorkspaceDraftIssue | TIssue>
) => Promise<TWorkspaceDraftIssue | undefined>;
deleteIssue: (workspaceSlug: string, issueId: string) => Promise<void>;
moveIssue: (workspaceSlug: string, issueId: string, payload: Partial<TWorkspaceDraftIssue>) => Promise<void>;
moveIssue: (workspaceSlug: string, issueId: string, payload: Partial<TWorkspaceDraftIssue>) => Promise<TIssue>;
addCycleToIssue: (
workspaceSlug: string,
issueId: string,
Expand Down Expand Up @@ -348,6 +350,10 @@ export class WorkspaceDraftIssues implements IWorkspaceDraftIssues {
total_count: this.paginationInfo.total_count - 1,
});
}

// sync issue to local db
addIssueToPersistanceLayer(response);

Comment on lines +353 to +356
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for persistence layer sync.

While the sync to persistence layer is correctly placed after the successful API response, it lacks error handling. Consider wrapping it in a try-catch block to prevent any persistence failures from affecting the UI.

        // sync issue to local db
-        addIssueToPersistanceLayer(response);
+        try {
+          await addIssueToPersistanceLayer(response);
+        } catch (error) {
+          // Log the error but don't throw to prevent UI disruption
+          console.error("Failed to sync issue to persistence layer:", error);
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// sync issue to local db
addIssueToPersistanceLayer(response);
// sync issue to local db
try {
await addIssueToPersistanceLayer(response);
} catch (error) {
// Log the error but don't throw to prevent UI disruption
console.error("Failed to sync issue to persistence layer:", error);
}

// Update draft issue count in workspaceUserInfo
this.updateWorkspaceUserDraftIssueCount(workspaceSlug, -1);
});
Expand Down