Skip to content

Commit

Permalink
refactor(web): drop issues context (#1470)
Browse files Browse the repository at this point in the history
Use TanStack to replace IssuesProvider. It is related to #1439 and
#1452.
  • Loading branch information
imobachgs authored Jul 17, 2024
2 parents fce3d4c + e45a059 commit 53eb89e
Show file tree
Hide file tree
Showing 17 changed files with 242 additions and 331 deletions.
2 changes: 2 additions & 0 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { useProduct, useProductChanges } from "./queries/software";
import { CONFIG, INSTALL, STARTUP } from "~/client/phase";
import { BUSY } from "~/client/status";
import { useL10nConfigChanges } from "~/queries/l10n";
import { useIssues, useIssuesChanges } from "./queries/issues";

/**
* Main application component.
Expand All @@ -45,6 +46,7 @@ function App() {
const { language } = useInstallerL10n();
useL10nConfigChanges();
useProductChanges();
useIssuesChanges();

const Content = () => {
if (error) return <ServerError />;
Expand Down
6 changes: 6 additions & 0 deletions web/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { STARTUP, CONFIG, INSTALL } from "~/client/phase";
import { IDLE, BUSY } from "~/client/status";
import { useL10nConfigChanges } from "./queries/l10n";
import { useProductChanges } from "./queries/software";
import { useIssuesChanges } from "./queries/issues";

jest.mock("~/client");

Expand All @@ -52,6 +53,11 @@ jest.mock("~/queries/l10n", () => ({
useL10nConfigChanges: () => jest.fn(),
}));

jest.mock("~/queries/issues", () => ({
...jest.requireActual("~/queries/issues"),
useIssuesChanges: () => jest.fn(),
}));

const mockClientStatus = {
connected: true,
error: false,
Expand Down
61 changes: 1 addition & 60 deletions web/src/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ import { HTTPClient, WSClient } from "./http";
* @property {UsersClient} users - users client.
* @property {QuestionsClient} questions - questions client.
* @property {() => WSClient} ws - Agama WebSocket client.
* @property {() => Promise<Issues>} issues - issues from all contexts.
* @property {(handler: IssuesHandler) => (() => void)} onIssuesChange - registers a handler to run
* when issues from any context change. It returns a function to deregister the handler.
* @property {() => boolean} isConnected - determines whether the client is connected
* @property {() => boolean} isRecoverable - determines whether the client is recoverable after disconnected
* @property {(handler: () => void) => (() => void)} onConnect - registers a handler to run
Expand All @@ -55,25 +52,6 @@ import { HTTPClient, WSClient } from "./http";
* handler.
*/

/**
* @typedef {import ("~/client/mixins").Issue} Issue
*
* @typedef {object} Issues
* @property {Issue[]} [product] - Issues from product.
* @property {Issue[]} [storage] - Issues from storage.
* @property {Issue[]} [software] - Issues from software.
* @property {Issue[]} [users] - Issues from users.
* @property {boolean} [isEmpty] - Whether the list is empty
*
* @typedef {(issues: Issues) => void} IssuesHandler
*/

const createIssuesList = (product = [], software = [], storage = [], users = []) => {
const list = { product, storage, software, users };
list.isEmpty = !Object.values(list).some((v) => v.length > 0);
return list;
};

/**
* Creates the Agama client
*
Expand All @@ -93,41 +71,6 @@ const createClient = (url) => {
const users = new UsersClient(client);
const questions = new QuestionsClient(client);

/**
* Gets all issues, grouping them by context.
*
* TODO: issues are requested by several components (e.g., overview sections, notifications
* provider, issues page, storage page, etc). There should be an issues provider.
*
* @returns {Promise<Issues>}
*/
const issues = async () => {
const productIssues = await product.getIssues();
const storageIssues = await storage.getIssues();
const softwareIssues = await software.getIssues();
const usersIssues = await users.getIssues();
return createIssuesList(productIssues, softwareIssues, storageIssues, usersIssues);
};

/**
* Registers a callback to be executed when issues change.
*
* @param {IssuesHandler} handler - Callback function.
* @return {() => void} - Function to deregister the callback.
*/
const onIssuesChange = (handler) => {
const unsubscribeCallbacks = [];

unsubscribeCallbacks.push(product.onIssuesChange((i) => handler({ product: i })));
unsubscribeCallbacks.push(storage.onIssuesChange((i) => handler({ storage: i })));
unsubscribeCallbacks.push(software.onIssuesChange((i) => handler({ software: i })));
unsubscribeCallbacks.push(users.onIssuesChange((i) => handler({ users: i })));

return () => {
unsubscribeCallbacks.forEach((cb) => cb());
};
};

const isConnected = () => client.ws().isConnected() || false;
const isRecoverable = () => !!client.ws().isRecoverable();

Expand All @@ -141,8 +84,6 @@ const createClient = (url) => {
storage,
users,
questions,
issues,
onIssuesChange,
isConnected,
isRecoverable,
onConnect: (handler) => client.ws().onOpen(handler),
Expand All @@ -157,4 +98,4 @@ const createDefaultClient = async () => {
return createClient(httpUrl);
};

export { createClient, createDefaultClient, phase, createIssuesList };
export { createClient, createDefaultClient, phase };
53 changes: 1 addition & 52 deletions web/src/client/mixins.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,57 +72,6 @@ const buildIssue = ({ description, details, source, severity }) => {
};
};

/**
* Extends the given class with methods to get the issues over D-Bus
*
* @template {!WithHTTPClient} T
* @param {T} superclass - superclass to extend
* @param {string} issues_path - validation resource path (e.g., "/manager/issues").
* @param {string} service_name - service name (e.g., "org.opensuse.Agama.Manager1").
*/
const WithIssues = (superclass, issues_path, service_name) =>
class extends superclass {
/**
* Returns the issues
*
* @return {Promise<Issue[]>}
*/
async getIssues() {
const response = await this.client.get(issues_path);
if (!response.ok) {
console.log("get issues failed with:", response);
return [];
} else {
const issues = await response.json();
return issues.map(buildIssue);
}
}

/**
* Gets all issues with error severity
*
* @return {Promise<Issue[]>}
*/
async getErrors() {
const issues = await this.getIssues();
return issues.filter((i) => i.severity === "error");
}

/**
* Registers a callback to run when the issues change
*
* @param {IssuesHandler} handler - callback function
* @return {import ("./http").RemoveFn} function to disable the callback
*/
onIssuesChange(handler) {
return this.client.onEvent("IssuesChanged", ({ service, issues }) => {
if (service === service_name) {
handler(issues.map(buildIssue));
}
});
}
};

/**
* Extends the given class with methods to get and track the service status
*
Expand Down Expand Up @@ -265,4 +214,4 @@ const createError = (message) => {
return { message };
};

export { WithIssues, WithProgress, WithStatus };
export { WithProgress, WithStatus };
23 changes: 6 additions & 17 deletions web/src/client/software.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@

// @ts-check

import { WithIssues, WithProgress, WithStatus } from "./mixins";
import { WithProgress, WithStatus } from "./mixins";

const SOFTWARE_SERVICE = "org.opensuse.Agama.Software1";
const PRODUCT_PATH = "/org/opensuse/Agama/Software1/Product";

/**
* Enum for the reasons to select a pattern
Expand Down Expand Up @@ -185,17 +184,13 @@ class SoftwareBaseClient {
/**
* Manages software and product configuration.
*/
class SoftwareClient extends WithIssues(
WithProgress(
WithStatus(SoftwareBaseClient, "/software/status", SOFTWARE_SERVICE),
"/software/progress",
SOFTWARE_SERVICE,
),
"/software/issues/software",
"/org/opensuse/Agama/Software1",
class SoftwareClient extends WithProgress(
WithStatus(SoftwareBaseClient, "/software/status", SOFTWARE_SERVICE),
"/software/progress",
SOFTWARE_SERVICE,
) {}

class ProductBaseClient {
class ProductClient {
/**
* @param {import("./http").HTTPClient} client - HTTP client.
*/
Expand Down Expand Up @@ -336,10 +331,4 @@ class ProductBaseClient {
}
}

class ProductClient extends WithIssues(
ProductBaseClient,
"/software/issues/product",
PRODUCT_PATH,
) {}

export { ProductClient, SelectedBy, SoftwareClient };
12 changes: 4 additions & 8 deletions web/src/client/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// cspell:ignore ptable

import { compact, hex, uniq } from "~/utils";
import { WithIssues, WithProgress, WithStatus } from "./mixins";
import { WithProgress, WithStatus } from "./mixins";
import { HTTPClient } from "./http";

const SERVICE_NAME = "org.opensuse.Agama.Storage1";
Expand Down Expand Up @@ -1652,13 +1652,9 @@ class StorageBaseClient {
/**
* Allows interacting with the storage settings
*/
class StorageClient extends WithIssues(
WithProgress(
WithStatus(StorageBaseClient, "/storage/status", SERVICE_NAME),
"/storage/progress",
SERVICE_NAME,
),
"/storage/issues",
class StorageClient extends WithProgress(
WithStatus(StorageBaseClient, "/storage/status", SERVICE_NAME),
"/storage/progress",
SERVICE_NAME,
) {}

Expand Down
72 changes: 0 additions & 72 deletions web/src/client/storage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1346,78 +1346,6 @@ describe.skip("#onDeprecate", () => {
});
});

describe("#getIssues", () => {
beforeEach(() => {
client = new StorageClient(http);
});

describe("if there are no issues", () => {
beforeEach(() => {
mockJsonFn.mockResolvedValue([]);
});

it("returns an empty list", async () => {
const issues = await client.getIssues();
expect(issues).toEqual([]);
});
});

describe("if there are issues", () => {
beforeEach(() => {
mockJsonFn.mockResolvedValue(contexts.withIssues());
});

it("returns the list of issues", async () => {
const issues = await client.getIssues();
expect(issues).toEqual(
expect.arrayContaining([
{ description: "Issue 1", details: "", source: "system", severity: "error" },
{ description: "Issue 2", details: "", source: "system", severity: "warn" },
{ description: "Issue 3", details: "", source: "config", severity: "error" },
]),
);
});
});
});

describe("#getErrors", () => {
beforeEach(() => {
client = new StorageClient(http);
mockJsonFn.mockResolvedValue(contexts.withIssues());
});

it("returns the issues with error severity", async () => {
const errors = await client.getErrors();
expect(errors.map((e) => e.description)).toEqual(
expect.arrayContaining(["Issue 1", "Issue 3"]),
);
});
});

// @fixme See note at the test of onDeprecate about mocking signals
describe.skip("#onIssuesChange", () => {
it("runs the handler when the issues change", async () => {
client = new StorageClient();

const handler = jest.fn();
client.onIssuesChange(handler);

emitSignal("/org/opensuse/Agama/Storage1", "org.opensuse.Agama1.Issues", {
All: {
v: [
["Issue 1", "", 1, 0],
["Issue 2", "", 2, 1],
],
},
});

expect(handler).toHaveBeenCalledWith([
{ description: "Issue 1", details: "", source: "system", severity: "warn" },
{ description: "Issue 2", details: "", source: "config", severity: "error" },
]);
});
});

describe("#system", () => {
describe("#getDevices", () => {
beforeEach(() => {
Expand Down
11 changes: 2 additions & 9 deletions web/src/client/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

// @ts-check

import { WithIssues } from "./mixins";

const SERVICE_NAME = "org.opensuse.Agama.Manager1";

/**
Expand All @@ -48,11 +46,11 @@ const SERVICE_NAME = "org.opensuse.Agama.Manager1";
*/

/**
* Users client
* Client to interact with the Agama users service
*
* @ignore
*/
class UsersBaseClient {
class UsersClient {
/**
* @param {import("./http").HTTPClient} client - HTTP client.
*/
Expand Down Expand Up @@ -183,9 +181,4 @@ class UsersBaseClient {
}
}

/**
* Client to interact with the Agama users service
*/
class UsersClient extends WithIssues(UsersBaseClient, "/users/issues", SERVICE_NAME) {}

export { UsersClient };
Loading

0 comments on commit 53eb89e

Please sign in to comment.