Skip to content
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.

Sign up for GitHub

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

[WEB-2001] feat: Fix local cache issues v2 #5712

Merged
merged 4 commits into from
Sep 27, 2024
Merged

[WEB-2001] feat: Fix local cache issues v2 #5712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be8f069
- Handle single quotes in load workspace queries
SatishGandham Sep 26, 2024
215889c
Fix description_html being lost
SatishGandham Sep 26, 2024
00c48c3
Change secondary order to sequence_id
SatishGandham Sep 27, 2024
5cece0f
Fix update persistence layer
SatishGandham Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions web/core/local-db/utils/indexes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const createIssueIndexes = async () => {
"project_id",
"created_by",
"cycle_id",
"sequence_id",
];

const promises: Promise<any>[] = [];
Expand Down
4 changes: 2 additions & 2 deletions web/core/local-db/utils/load-workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const stageInserts = (table: string, schema: Schema, data: any) => {
return "";
}
if (typeof value === "object") {
return `'${JSON.stringify(value)}'`;
return `'${JSON.stringify(value).replace(/'/g, "''")}'`;
}
if (typeof value === "string") {
return `'${value}'`;
return `'${value.replace(/'/g, "''")}'`;
}
return value;
})
Expand Down
17 changes: 8 additions & 9 deletions web/core/local-db/utils/query.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export const getOrderByFragment = (order_by: string, table = "") => {
if (!order_by) return orderByString;

if (order_by.startsWith("-")) {
orderByString += ` ORDER BY ${wrapDateTime(order_by.slice(1))} DESC NULLS LAST, datetime(${table}created_at) DESC`;
orderByString += ` ORDER BY ${wrapDateTime(order_by.slice(1))} DESC NULLS LAST, ${table}sequence_id DESC`;
} else {
orderByString += ` ORDER BY ${wrapDateTime(order_by)} ASC NULLS LAST, datetime(${table}created_at) DESC`;
orderByString += ` ORDER BY ${wrapDateTime(order_by)} ASC NULLS LAST, ${table}sequence_id DESC`;
}
return orderByString;
};
Expand Down Expand Up @@ -130,7 +130,7 @@ export const getFilteredRowsForGrouping = (projectId: string, queries: any) => {

let sql = "";
if (!joinsRequired) {
sql = `WITH fi as (SELECT i.id,i.created_at ${issueTableFilterFields}`;
sql = `WITH fi as (SELECT i.id,i.created_at, i.sequence_id ${issueTableFilterFields}`;
if (group_by) {
if (group_by === "target_date") {
sql += `, date(i.${group_by}) as group_id`;
Expand All @@ -153,7 +153,7 @@ export const getFilteredRowsForGrouping = (projectId: string, queries: any) => {
}

sql = `WITH fi AS (`;
sql += `SELECT i.id,i.created_at ${issueTableFilterFields} `;
sql += `SELECT i.id,i.created_at,i.sequence_id ${issueTableFilterFields} `;
if (group_by) {
if (ARRAY_FIELDS.includes(group_by)) {
sql += `, ${group_by}.value as group_id
Expand Down Expand Up @@ -238,7 +238,10 @@ export const singleFilterConstructor = (queries: any) => {

keys.forEach((key) => {
const value = filters[key] ? filters[key].split(",") : "";
if (!value) return;
if (!value) {
sql += ` AND ${key} IS NULL`;
return;
}
Comment on lines +241 to +244
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

Potential issue with null value handling

The addition of null value handling is a good improvement. However, the current implementation might lead to incorrect SQL generation for array fields. The condition if (!value) will be true for both empty strings and empty arrays, which could cause issues for array fields checked later in the function.

Consider refining the condition to specifically check for empty strings:

- if (!value) {
+ if (value === '') {
    sql += ` AND ${key} IS NULL`;
    return;
  }

This change ensures that only empty string values are treated as NULL in the SQL query, while empty arrays are handled correctly by the existing logic for array fields.

📝 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 (!value) {
sql += ` AND ${key} IS NULL`;
return;
}
if (value === '') {
sql += ` AND ${key} IS NULL`;
return;
}

if (!ARRAY_FIELDS.includes(key)) {
sql += ` AND ${key} in ('${value.join("','")}')
`;
Expand All @@ -249,10 +252,6 @@ export const singleFilterConstructor = (queries: any) => {
return sql;
};

// let q = '2_months;after;fromnow,1_months;after;fromnow,2024-09-01;after,2024-10-06;after,2_weeks;after;fromnow'

// ["2_months;after;fromnow", "1_months;after;fromnow", "2024-09-01;after", "2024-10-06;before", "2_weeks;after;fromnow"];

const createDateFilter = (key: string, q: string) => {
let sql = " ";
// get todays date in YYYY-MM-DD format
Expand Down
8 changes: 6 additions & 2 deletions web/core/local-db/utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pick from "lodash/pick";
import { TIssue } from "@plane/types";
import { rootStore } from "@/lib/store-context";
import { persistence } from "../storage.sqlite";
import { updateIssue } from "./load-issues";

export const log = (...args: any) => {
Expand All @@ -15,11 +16,13 @@ export const updatePersistentLayer = async (issueIds: string | string[]) => {
if (typeof issueIds === "string") {
issueIds = [issueIds];
}
issueIds.forEach((issueId) => {
issueIds.forEach(async (issueId) => {
const dbIssue = await persistence.getIssue(issueId);
const issue = rootStore.issue.issues.getIssueById(issueId);

if (issue) {
const issuePartial = pick(JSON.parse(JSON.stringify(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",
Expand Down Expand Up @@ -47,6 +50,7 @@ export const updatePersistentLayer = async (issueIds: string | string[]) => {
"label_ids",
"module_ids",
"type_id",
"description_html",
]);
updateIssue({ ...issuePartial, is_local_update: 1 });
}
Expand Down
Loading