Skip to content

Commit

Permalink
Query parsing error handling (#53)
Browse files Browse the repository at this point in the history
  • Loading branch information
jamiebrynes7 authored Oct 19, 2020
1 parent 42ab814 commit 48b2274
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
}
```
- You can now check for plugin updates in the Todoist plugin settings.
- The JSON query is now validated and errors are presented in a more user-friendly fashion:
![Query parsing error example](./assets/query-parsing-error-example.png)

### 🔃 Changed

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ An experimental [Obsidian](https://obsidian.md/) plugin using [Volcano](https://

_Tested with Obsidian 0.8.14 and Volcano 1.2.1, your results may vary!_

![Example gif](./.github/obsidian-todoist-sync.gif)
![Example gif](./assets/obsidian-todoist-sync.gif)

## Usage

Expand Down
File renamed without changes
Binary file added assets/query-parsing-error-example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
33 changes: 23 additions & 10 deletions src/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import TodoistQuery from "./ui/TodoistQuery.svelte";
import type IQuery from "./query";
import ErrorDisplay from "./ui/ErrorDisplay.svelte";
import { parseQuery } from "./query";
import { SettingsInstance, ISettings, SettingsTab } from "./settings";
import { TodoistApi } from "./api/api";
import type { App, Settings, PluginInstance } from "./obsidian";
import debug from "./log";
import type SvelteComponentDev from "./ui/TodoistQuery.svelte";

interface IInjection {
component: TodoistQuery;
component: SvelteComponentDev;
workspaceLeaf: Node;
}

Expand Down Expand Up @@ -163,7 +165,7 @@ export default class TodoistPlugin<TBase extends Settings> {
});

// TODO: Error handling.
const query = JSON.parse(node.innerText) as IQuery;
const query = parseQuery(JSON.parse(node.innerText));

debug({
msg: "Parsed query",
Expand All @@ -173,13 +175,24 @@ export default class TodoistPlugin<TBase extends Settings> {
const root = node.parentElement;
root.removeChild(node);

const queryNode = new TodoistQuery({
target: root,
props: {
query: query,
api: this.api,
},
});
let queryNode: SvelteComponentDev = null;

if (query.isOk()) {
queryNode = new TodoistQuery({
target: root,
props: {
query: query.unwrap(),
api: this.api,
},
});
} else {
queryNode = new ErrorDisplay({
target: root,
props: {
error: query.unwrapErr(),
},
});
}

const injection = {
component: queryNode,
Expand Down
54 changes: 54 additions & 0 deletions src/query.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,61 @@
import { Result } from "./result";

export default interface IQuery {
name: string;
filter: string;
autorefresh?: number;
sorting?: string[];
group: boolean;
}

const possibleSortingOptions = ["date", "priority"];

export function parseQuery(query: any): Result<IQuery, Error> {
if (!query.hasOwnProperty("name")) {
return Result.Err(new Error("Missing 'name' field in query."));
}

if (!query.hasOwnProperty("filter")) {
return Result.Err(new Error("Missing 'filter' field in query"));
}

if (
query.hasOwnProperty("autorefresh") &&
(isNaN(query.autorefresh) || (query.autorefresh as number) < 0)
) {
return Result.Err(
new Error("'autorefresh' field must be a positive number.")
);
}

if (query.hasOwnProperty("sorting")) {
if (!Array.isArray(query.sorting)) {
return Result.Err(
new Error(
"'sorting' field must be an array of strings within the set ['date', 'priority']."
)
);
}

const sorting = query.sorting as any[];

for (const element of sorting) {
if (
!(typeof element == "string") ||
possibleSortingOptions.indexOf(element as string) == -1
) {
return Result.Err(
new Error(
"'sorting' field must be an array of strings within the set ['date', 'priority']."
)
);
}
}
}

if (query.hasOwnProperty("group") && typeof query.group != "boolean") {
return Result.Err(new Error("'group' field must be a boolean."));
}

return Result.Ok(query as IQuery);
}
2 changes: 0 additions & 2 deletions tests/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import type { ITaskRaw, IProjectRaw, ISectionRaw } from "../src/api/raw_models";
import type { ITodoistMetadata } from "../src/api/api";
import { ExtendedMap } from "../src/utils";

// Unknown projects

describe("Project tree parsing", () => {
it("Projects are parented correctly", () => {
const rawTasks: ITaskRaw[] = [
Expand Down
126 changes: 126 additions & 0 deletions tests/query_parsing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import "mocha";
import { assert } from "chai";
import { parseQuery } from "../src/query";

describe("Query parsing", () => {
it("Name must exist", () => {
const obj = {
filter: "foo",
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());
});

it("Filter must exist", () => {
const obj = {
name: "Tasks",
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());
});

it("Only name & filter are required", () => {
const obj = {
name: "Tasks",
filter: "foo",
};

const result = parseQuery(obj);
assert.isTrue(result.isOk());
});

it("Autorefresh must be a number", () => {
const obj = {
name: "Tasks",
filter: "foo",
autorefresh: "NaN",
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());
});

it("Autorefresh cannot be negative", () => {
const obj = {
name: "Tasks",
filter: "foo",
autorefresh: -1,
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());
});

it("Valid autorefresh values pass", () => {
const obj = {
name: "Tasks",
filter: "foo",
autorefresh: 1,
};

const result = parseQuery(obj);
assert.isTrue(result.isOk());
});

it("Sorting must be an array", () => {
const obj = {
name: "Tasks",
filter: "foo",
sorting: "Not an array",
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());

const other = {
name: "Tasks",
filter: "foo",
sorting: [],
};

const otherResult = parseQuery(other);
assert.isTrue(otherResult.isOk());
});

it("Sorting can only be a specified set of options", () => {
const obj = {
name: "Tasks",
filter: "filter",
sorting: ["not-valid"],
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());

const validObj = {
name: "Tasks",
filter: "filter",
sorting: ["date", "priority"],
};

const otherResult = parseQuery(validObj);
assert.isTrue(otherResult.isOk());
});

it("Group must be a boolean", () => {
const obj = {
name: "Tasks",
filter: "Filter",
group: "not-a-boolean",
};

const result = parseQuery(obj);
assert.isTrue(result.isErr());

const validObj = {
name: "Tasks",
filter: "Filter",
group: true,
};

const otherResult = parseQuery(validObj);
assert.isTrue(otherResult.isOk());
});
});

0 comments on commit 48b2274

Please sign in to comment.