From 809b50dd6ec8a256ba245dec708bc7f75e1f8edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 30 Jul 2024 12:34:57 +0100 Subject: [PATCH 01/10] refactor(web): add basic types for questions --- web/src/types/questions.ts | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 web/src/types/questions.ts diff --git a/web/src/types/questions.ts b/web/src/types/questions.ts new file mode 100644 index 0000000000..85c88fe984 --- /dev/null +++ b/web/src/types/questions.ts @@ -0,0 +1,50 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +/** + * Enum for question types + */ +enum QuestionType { + generic = "generic", + withPassword = "withPassword", +} + +type Question = { + id: number; + type?: QuestionType; + class?: string; + options?: string[]; + defaultOption?: string; + text?: string; + data?: { [key: string]: string }; + answer?: string; + password?: string; +}; + +type Answer = { + generic?: { answer: string }; + withPassword?: { password: string }; +}; + +type AnswerCallback = (answeredQuestion: Question) => void; + +export { QuestionType }; +export type { Answer, AnswerCallback, Question }; From 4d67bb4c8655440ce8dd828c69535561b6a9d174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 30 Jul 2024 12:35:22 +0100 Subject: [PATCH 02/10] refactor(web): add questions queries --- web/src/queries/questions.ts | 119 +++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 web/src/queries/questions.ts diff --git a/web/src/queries/questions.ts b/web/src/queries/questions.ts new file mode 100644 index 0000000000..118fd0e029 --- /dev/null +++ b/web/src/queries/questions.ts @@ -0,0 +1,119 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useInstallerClient } from "~/context/installer"; +import { Answer, Question, QuestionType } from "~/types/questions"; + +type APIQuestion = { + generic?: Question; + withPassword?: Pick; +}; + +/** + * Internal method to build proper question objects + * + * TODO: improve/simplify it once the backend API is improved. + */ +function buildQuestion(httpQuestion: APIQuestion) { + let question: Question; + + if (httpQuestion.generic) { + question = { + ...httpQuestion.generic, + type: QuestionType.generic, + answer: httpQuestion.generic.answer, + }; + } + + if (httpQuestion.withPassword) { + question = { + id: httpQuestion.generic.id, + type: QuestionType.withPassword, + password: httpQuestion.withPassword.password, + }; + } + + return question; +} + +/** + * Query to retrieve questions + */ +const questionsQuery = () => ({ + queryKey: ["questions"], + queryFn: () => fetch("/api/questions").then((res) => res.json()), +}); + +/** + * Hook that builds a mutation given question, allowing to answer it + + * TODO: improve/simplify it once the backend API is improved. + */ +const useQuestionsConfig = () => { + const query = { + mutationFn: (question: Question) => { + const answer: Answer = { generic: { answer: question.answer } }; + + if (question.type === QuestionType.withPassword) { + answer.withPassword = { password: question.password }; + } + + return fetch(`/api/questions/${question.id}/answer`, { + method: "PATCH", + body: JSON.stringify(answer), + headers: { + "Content-Type": "application/json", + }, + }); + }, + }; + return useMutation(query); +}; + +/** + * Hook for listening questions changes and performing proper invalidations + */ +const useQuestionsChanges = () => { + const queryClient = useQueryClient(); + const client = useInstallerClient(); + + React.useEffect(() => { + if (!client) return; + + return client.ws().onEvent((event) => { + if (event.type === "QuestionsChanged") { + queryClient.invalidateQueries({ queryKey: ["questions"] }); + } + }); + }, [client, queryClient]); +}; + +/** + * Hook for retrieving available questions + */ +const useQuestions = () => { + const { data, isPending } = useQuery(questionsQuery()); + return isPending ? [] : data.map(buildQuestion); +}; + +export { questionsQuery, useQuestions, useQuestionsConfig, useQuestionsChanges }; From 150bd517f3fd8d97fdaaeaf88993b1e6b8643971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 30 Jul 2024 12:35:46 +0100 Subject: [PATCH 03/10] refactor(web): adapt question components to queries --- .../questions/GenericQuestion.test.jsx | 4 +- .../questions/LuksActivationQuestion.test.jsx | 4 +- .../questions/QuestionWithPassword.test.jsx | 4 +- web/src/components/questions/Questions.jsx | 61 +++------ .../components/questions/Questions.test.jsx | 125 +++++++++--------- 5 files changed, 82 insertions(+), 116 deletions(-) diff --git a/web/src/components/questions/GenericQuestion.test.jsx b/web/src/components/questions/GenericQuestion.test.jsx index ef3f3f176d..6573af614f 100644 --- a/web/src/components/questions/GenericQuestion.test.jsx +++ b/web/src/components/questions/GenericQuestion.test.jsx @@ -21,7 +21,7 @@ import React from "react"; import { screen } from "@testing-library/react"; -import { installerRender } from "~/test-utils"; +import { plainRender } from "~/test-utils"; import { GenericQuestion } from "~/components/questions"; const question = { @@ -34,7 +34,7 @@ const question = { const answerFn = jest.fn(); const renderQuestion = () => - installerRender(); + plainRender(); describe("GenericQuestion", () => { it("renders the question text", async () => { diff --git a/web/src/components/questions/LuksActivationQuestion.test.jsx b/web/src/components/questions/LuksActivationQuestion.test.jsx index 6bee9e3754..8c56815873 100644 --- a/web/src/components/questions/LuksActivationQuestion.test.jsx +++ b/web/src/components/questions/LuksActivationQuestion.test.jsx @@ -21,14 +21,14 @@ import React from "react"; import { screen } from "@testing-library/react"; -import { installerRender } from "~/test-utils"; +import { plainRender } from "~/test-utils"; import { LuksActivationQuestion } from "~/components/questions"; let question; const answerFn = jest.fn(); const renderQuestion = () => - installerRender(); + plainRender(); describe("LuksActivationQuestion", () => { beforeEach(() => { diff --git a/web/src/components/questions/QuestionWithPassword.test.jsx b/web/src/components/questions/QuestionWithPassword.test.jsx index c9831d72d0..bedcbb3cb1 100644 --- a/web/src/components/questions/QuestionWithPassword.test.jsx +++ b/web/src/components/questions/QuestionWithPassword.test.jsx @@ -49,12 +49,12 @@ describe("QuestionWithPassword", () => { }); describe("when the user enters the password", () => { - it("calls the callback", async () => { + it("calls the callback with given password", async () => { const { user } = renderQuestion(); const passwordInput = await screen.findByLabelText("Password"); await user.type(passwordInput, "notSecret"); - const skipButton = await screen.findByRole("button", { name: /Ok/ }); + const skipButton = await screen.findByRole("button", { name: "Ok" }); await user.click(skipButton); expect(question).toEqual(expect.objectContaining({ password: "notSecret", answer: "ok" })); diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 4280a4cf6b..1da02c97a6 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -19,73 +19,42 @@ * find current contact information at www.suse.com. */ -import React, { useCallback, useEffect, useState } from "react"; -import { useInstallerClient } from "~/context/installer"; -import { useCancellablePromise } from "~/utils"; -import { QUESTION_TYPES } from "~/client/questions"; +// @ts-check +import React from "react"; import { GenericQuestion, QuestionWithPassword, LuksActivationQuestion, } from "~/components/questions"; +import { useQuestions, useQuestionsConfig, useQuestionsChanges } from "~/queries/questions"; +import { Question, QuestionType } from "~/types/questions"; export default function Questions() { - const client = useInstallerClient(); - const { cancellablePromise } = useCancellablePromise(); - - const [pendingQuestions, setPendingQuestions] = useState([]); - - const addQuestion = useCallback((question) => { - setPendingQuestions((pending) => [...pending, question]); - }, []); - - const removeQuestion = useCallback( - (id) => setPendingQuestions((pending) => pending.filter((q) => q.id !== id)), - [], - ); - - const answerQuestion = useCallback( - (question) => { - client.questions.answer(question); - removeQuestion(question.id); - }, - [client.questions, removeQuestion], - ); - - useEffect(() => { - client.questions.listenQuestions(); - }, [client.questions, cancellablePromise]); - - useEffect(() => { - cancellablePromise(client.questions.getQuestions()) - .then(setPendingQuestions) - .catch((e) => console.error("Something went wrong retrieving pending questions", e)); - }, [client.questions, cancellablePromise]); - - useEffect(() => { - const unsubscribeCallbacks = []; - unsubscribeCallbacks.push(client.questions.onQuestionAdded(addQuestion)); - unsubscribeCallbacks.push(client.questions.onQuestionRemoved(removeQuestion)); - - return () => { - unsubscribeCallbacks.forEach((cb) => cb()); - }; - }, [client.questions, addQuestion, removeQuestion]); + useQuestionsChanges(); + const pendingQuestions = useQuestions(); + const questionsConfig = useQuestionsConfig(); if (pendingQuestions.length === 0) return null; + const answerQuestion = (/** @type {Question} */ answeredQuestion) => + questionsConfig.mutate(answeredQuestion); + // Renders the first pending question const [currentQuestion] = pendingQuestions; + let QuestionComponent = GenericQuestion; + // show specialized popup for question which need password - if (currentQuestion.type === QUESTION_TYPES.withPassword) { + if (currentQuestion.type === QuestionType.withPassword) { QuestionComponent = QuestionWithPassword; } + // show specialized popup for luks activation question // more can follow as it will be needed if (currentQuestion.class === "storage.luks_activation") { QuestionComponent = LuksActivationQuestion; } + return ; } diff --git a/web/src/components/questions/Questions.test.jsx b/web/src/components/questions/Questions.test.jsx index e951a853f9..02a2459d58 100644 --- a/web/src/components/questions/Questions.test.jsx +++ b/web/src/components/questions/Questions.test.jsx @@ -20,107 +20,104 @@ */ import React from "react"; - -import { act, waitFor, within } from "@testing-library/react"; -import { installerRender } from "~/test-utils"; -import { createClient } from "~/client"; - +import { screen } from "@testing-library/react"; +import { installerRender, plainRender } from "~/test-utils"; import { Questions } from "~/components/questions"; +import { QuestionType } from "~/types/questions"; +import * as GenericQuestionComponent from "~/components/questions/GenericQuestion"; + +let mockQuestions; +const mockMutation = jest.fn(); -jest.mock("~/client"); -jest.mock("~/components/questions/GenericQuestion", () => () =>
A Generic question mock
); jest.mock("~/components/questions/LuksActivationQuestion", () => () => (
A LUKS activation question mock
)); +jest.mock("~/components/questions/QuestionWithPassword", () => () => ( +
A question with password mock
+)); -const handlers = {}; -const genericQuestion = { id: 1, type: "generic" }; +jest.mock("~/queries/questions", () => ({ + ...jest.requireActual("~/queries/software"), + useQuestions: () => mockQuestions, + useQuestionsChanges: () => jest.fn(), + useQuestionsConfig: () => ({ mutate: mockMutation }), +})); + +const genericQuestion = { + id: 1, + type: QuestionType.generic, + text: "Do you write unit tests?", + options: ["always", "sometimes", "never"], + defaultOption: "sometimes", +}; +const passwordQuestion = { id: 1, type: QuestionType.withPassword }; const luksActivationQuestion = { id: 1, class: "storage.luks_activation" }; -let pendingQuestions = []; - -beforeEach(() => { - createClient.mockImplementation(() => { - return { - questions: { - getQuestions: () => Promise.resolve(pendingQuestions), - // Capture the handler for the onQuestionAdded signal for triggering it manually - onQuestionAdded: (onAddHandler) => { - handlers.onAdd = onAddHandler; - return jest.fn; - }, - // Capture the handler for the onQuestionREmoved signal for triggering it manually - onQuestionRemoved: (onRemoveHandler) => { - handlers.onRemove = onRemoveHandler; - return jest.fn; - }, - listenQuestions: jest.fn(), - }, - }; - }); -}); describe("Questions", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + describe("when there are no pending questions", () => { beforeEach(() => { - pendingQuestions = []; + mockQuestions = []; }); - it("renders nothing", async () => { - const { container } = installerRender(); - await waitFor(() => expect(container).toBeEmptyDOMElement()); + it("renders nothing", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); }); }); - describe("when a new question is added", () => { - it("push it into the pending queue", async () => { - const { container } = installerRender(); - await waitFor(() => expect(container).toBeEmptyDOMElement()); - - // Manually triggers the handler given for the onQuestionAdded signal - act(() => handlers.onAdd(genericQuestion)); + describe("when a question is answered", () => { + beforeEach(() => { + mockQuestions = [genericQuestion]; + }); - await within(container).findByText("A Generic question mock"); + it("triggers the useQuestionMutationk", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: "Always" }); + await user.click(button); + expect(mockMutation).toHaveBeenCalledWith({ ...genericQuestion, answer: "always" }); }); }); - describe("when a question is removed", () => { + describe("when there is a generic question pending", () => { beforeEach(() => { - pendingQuestions = [genericQuestion]; + mockQuestions = [genericQuestion]; + // Not using jest.mock at the top like for the other question components + // because the original implementation was needed for testing that + // mutation is triggered when proceed. + jest + .spyOn(GenericQuestionComponent, "default") + .mockReturnValue(
A generic question mock
); }); - it("removes it from the queue", async () => { - const { container } = installerRender(); - await within(container).findByText("A Generic question mock"); - - // Manually triggers the handler given for the onQuestionRemoved signal - act(() => handlers.onRemove(genericQuestion.id)); - - const content = within(container).queryByText("A Generic question mock"); - expect(content).toBeNull(); + it("renders a GenericQuestion component", () => { + plainRender(); + screen.getByText("A generic question mock"); }); }); describe("when there is a generic question pending", () => { beforeEach(() => { - pendingQuestions = [genericQuestion]; + mockQuestions = [passwordQuestion]; }); - it("renders a GenericQuestion component", async () => { - const { container } = installerRender(); - - await within(container).findByText("A Generic question mock"); + it("renders a QuestionWithPassword component", () => { + plainRender(); + screen.getByText("A question with password mock"); }); }); describe("when there is a LUKS activation question pending", () => { beforeEach(() => { - pendingQuestions = [luksActivationQuestion]; + mockQuestions = [luksActivationQuestion]; }); - it("renders a LuksActivationQuestion component", async () => { - const { container } = installerRender(); - - await within(container).findByText("A LUKS activation question mock"); + it("renders a LuksActivationQuestion component", () => { + installerRender(); + screen.getByText("A LUKS activation question mock"); }); }); }); From f42aa70d5c72d3683726920d139b80b505e3072c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 30 Jul 2024 12:36:13 +0100 Subject: [PATCH 04/10] refactor(web): drop questions client Since it has been replaced by queries. --- web/src/client/index.js | 4 - web/src/client/questions.js | 153 ------------------------------- web/src/client/questions.test.js | 124 ------------------------- 3 files changed, 281 deletions(-) delete mode 100644 web/src/client/questions.js delete mode 100644 web/src/client/questions.test.js diff --git a/web/src/client/index.js b/web/src/client/index.js index b1920f1f18..e3ca0a3010 100644 --- a/web/src/client/index.js +++ b/web/src/client/index.js @@ -27,7 +27,6 @@ import { Monitor } from "./monitor"; import { ProductClient, SoftwareClient } from "./software"; import { StorageClient } from "./storage"; import phase from "./phase"; -import { QuestionsClient } from "./questions"; import { NetworkClient } from "./network"; import { HTTPClient, WSClient } from "./http"; @@ -40,7 +39,6 @@ import { HTTPClient, WSClient } from "./http"; * @property {ProductClient} product - product client. * @property {SoftwareClient} software - software client. * @property {StorageClient} storage - storage client. - * @property {QuestionsClient} questions - questions client. * @property {() => WSClient} ws - Agama WebSocket client. * @property {() => boolean} isConnected - determines whether the client is connected * @property {() => boolean} isRecoverable - determines whether the client is recoverable after disconnected @@ -66,7 +64,6 @@ const createClient = (url) => { const network = new NetworkClient(client); const software = new SoftwareClient(client); const storage = new StorageClient(client); - const questions = new QuestionsClient(client); const isConnected = () => client.ws().isConnected() || false; const isRecoverable = () => !!client.ws().isRecoverable(); @@ -79,7 +76,6 @@ const createClient = (url) => { network, software, storage, - questions, isConnected, isRecoverable, onConnect: (handler) => client.ws().onOpen(handler), diff --git a/web/src/client/questions.js b/web/src/client/questions.js deleted file mode 100644 index d2e822c238..0000000000 --- a/web/src/client/questions.js +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Copyright (c) [2022-2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -const QUESTION_TYPES = { - generic: "generic", - withPassword: "withPassword", -}; - -/** - * @param {Object} httpQuestion - * @return {Object} - */ -function buildQuestion(httpQuestion) { - let question = {}; - if (httpQuestion.generic) { - question.type = QUESTION_TYPES.generic; - question = { ...httpQuestion.generic, type: QUESTION_TYPES.generic }; - question.answer = httpQuestion.generic.answer; - } - - if (httpQuestion.withPassword) { - question.type = QUESTION_TYPES.withPassword; - question.password = httpQuestion.withPassword.password; - } - - return question; -} - -/** - * Questions client - */ -class QuestionsClient { - /** - * @param {import("./http").HTTPClient} client - HTTP client. - */ - constructor(client) { - this.client = client; - this.listening = false; - this.questionIds = []; - this.handlers = { - added: [], - removed: [], - }; - } - - /** - * Return all the questions - * - * @return {Promise>} - */ - async getQuestions() { - const response = await this.client.get("/questions"); - if (!response.ok) { - console.warn("Failed to get questions: ", response); - return []; - } - const questions = await response.json(); - return questions.map(buildQuestion); - } - - /** - * Answer with the information in the given question - * - * @param {Object} question - */ - answer(question) { - const answer = { generic: { answer: question.answer } }; - if (question.type === QUESTION_TYPES.withPassword) { - answer.withPassword = { password: question.password }; - } - - const path = `/questions/${question.id}/answer`; - return this.client.put(path, answer); - } - - /** - * Register a callback to run when a questions is added - * - * @param {function} handler - callback function - * @return {function} function to unsubscribe - */ - onQuestionAdded(handler) { - this.handlers.added.push(handler); - - return () => { - const position = this.handlers.added.indexOf(handler); - if (position > -1) this.handlers.added.splice(position, 1); - }; - } - - /** - * Register a callback to run when a questions is removed - * - * @param {function} handler - callback function - * @return {function} function to unsubscribe - */ - onQuestionRemoved(handler) { - this.handlers.removed.push(handler); - - return () => { - const position = this.handlers.removed.indexOf(handler); - if (position > -1) this.handlers.removed.splice(position, 1); - }; - } - - async listenQuestions() { - if (this.listening) return; - - this.listening = true; - this.getQuestions().then((qs) => { - this.questionIds = qs.map((q) => q.id); - }); - return this.client.onEvent("QuestionsChanged", () => { - this.getQuestions().then((qs) => { - const updatedIds = qs.map((q) => q.id); - - const newQuestions = qs.filter((q) => !this.questionIds.includes(q.id)); - newQuestions.forEach((q) => { - this.handlers.added.forEach((f) => f(q)); - }); - - const removed = this.questionIds.filter((id) => !updatedIds.includes(id)); - removed.forEach((id) => { - this.handlers.removed.forEach((f) => f(id)); - }); - - this.questionIds = updatedIds; - }); - }); - } -} - -export { QUESTION_TYPES, QuestionsClient }; diff --git a/web/src/client/questions.test.js b/web/src/client/questions.test.js deleted file mode 100644 index 21835aced5..0000000000 --- a/web/src/client/questions.test.js +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Copyright (c) [2022-2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -import { HTTPClient } from "./http"; -import { QuestionsClient } from "./questions"; - -const mockJsonFn = jest.fn(); -const mockGetFn = jest.fn().mockImplementation(() => { - return { ok: true, json: mockJsonFn }; -}); -const mockPutFn = jest.fn().mockImplementation(() => { - return { ok: true }; -}); - -jest.mock("./http", () => { - return { - HTTPClient: jest.fn().mockImplementation(() => { - return { - get: mockGetFn, - put: mockPutFn, - onEvent: jest.fn(), - }; - }), - }; -}); - -let client; - -const expectedQuestions = [ - { - id: 432, - class: "storage.luks_activation", - type: "withPassword", - text: "The device /dev/vdb1 (2.00 GiB) is encrypted. Do you want to decrypt it?", - options: ["skip", "decrypt"], - defaultOption: "decrypt", - answer: "", - data: { Attempt: "1" }, - password: "", - }, -]; - -const luksQuestion = { - generic: { - id: 432, - class: "storage.luks_activation", - text: "The device /dev/vdb1 (2.00 GiB) is encrypted. Do you want to decrypt it?", - options: ["skip", "decrypt"], - defaultOption: "decrypt", - data: { Attempt: "1" }, - answer: "", - }, - withPassword: { password: "" }, -}; - -describe("#getQuestions", () => { - beforeEach(() => { - mockJsonFn.mockResolvedValue([luksQuestion]); - client = new QuestionsClient(new HTTPClient(new URL("http://localhost"))); - }); - - it("returns pending questions", async () => { - const questions = await client.getQuestions(); - expect(mockGetFn).toHaveBeenCalledWith("/questions"); - expect(questions).toEqual(expectedQuestions); - }); -}); - -describe("#answer", () => { - let question; - - beforeEach(() => { - question = { id: 321, type: "whatever", answer: "the-answer" }; - }); - - it("sets given answer", async () => { - client = new QuestionsClient(new HTTPClient(new URL("http://localhost"))); - await client.answer(question); - - expect(mockPutFn).toHaveBeenCalledWith("/questions/321/answer", { - generic: { answer: "the-answer" }, - }); - }); - - describe("when answering a question implementing the LUKS activation interface", () => { - beforeEach(() => { - question = { - id: 432, - type: "withPassword", - class: "storage.luks_activation", - answer: "decrypt", - password: "notSecret", - }; - }); - - it("sets given password", async () => { - client = new QuestionsClient(new HTTPClient(new URL("http://localhost"))); - await client.answer(question); - - expect(mockPutFn).toHaveBeenCalledWith("/questions/432/answer", { - generic: { answer: "decrypt" }, - withPassword: { password: "notSecret" }, - }); - }); - }); -}); From 560f953e9fac707688718fbddf8bf3e9dc249be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 31 Jul 2024 13:41:35 +0100 Subject: [PATCH 05/10] refactor(web): migrate components/questions to TypeScript --- ...tion.test.jsx => GenericQuestion.test.tsx} | 5 +- ...enericQuestion.jsx => GenericQuestion.tsx} | 19 +++++-- ...st.jsx => LuksActivationQuestion.test.tsx} | 53 ++++++------------- ...uestion.jsx => LuksActivationQuestion.tsx} | 28 +++++++--- ...ions.test.jsx => QuestionActions.test.tsx} | 5 +- ...uestionActions.jsx => QuestionActions.tsx} | 29 ++++++---- ...test.jsx => QuestionWithPassword.test.tsx} | 20 +++---- ...hPassword.jsx => QuestionWithPassword.tsx} | 19 +++++-- ...{Questions.test.jsx => Questions.test.tsx} | 12 ++--- .../{Questions.jsx => Questions.tsx} | 10 ++-- .../questions/{index.js => index.ts} | 2 +- 11 files changed, 111 insertions(+), 91 deletions(-) rename web/src/components/questions/{GenericQuestion.test.jsx => GenericQuestion.test.tsx} (95%) rename web/src/components/questions/{GenericQuestion.jsx => GenericQuestion.tsx} (73%) rename web/src/components/questions/{LuksActivationQuestion.test.jsx => LuksActivationQuestion.test.tsx} (77%) rename web/src/components/questions/{LuksActivationQuestion.jsx => LuksActivationQuestion.tsx} (76%) rename web/src/components/questions/{QuestionActions.test.jsx => QuestionActions.test.tsx} (97%) rename web/src/components/questions/{QuestionActions.jsx => QuestionActions.tsx} (74%) rename web/src/components/questions/{QuestionWithPassword.test.jsx => QuestionWithPassword.test.tsx} (87%) rename web/src/components/questions/{QuestionWithPassword.jsx => QuestionWithPassword.tsx} (78%) rename web/src/components/questions/{Questions.test.jsx => Questions.test.tsx} (91%) rename web/src/components/questions/{Questions.jsx => Questions.tsx} (88%) rename web/src/components/questions/{index.js => index.ts} (96%) diff --git a/web/src/components/questions/GenericQuestion.test.jsx b/web/src/components/questions/GenericQuestion.test.tsx similarity index 95% rename from web/src/components/questions/GenericQuestion.test.jsx rename to web/src/components/questions/GenericQuestion.test.tsx index 6573af614f..c5f3536688 100644 --- a/web/src/components/questions/GenericQuestion.test.jsx +++ b/web/src/components/questions/GenericQuestion.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -23,8 +23,9 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { GenericQuestion } from "~/components/questions"; +import { Question } from "~/types/questions"; -const question = { +const question: Question = { id: 1, text: "Do you write unit tests?", options: ["always", "sometimes", "never"], diff --git a/web/src/components/questions/GenericQuestion.jsx b/web/src/components/questions/GenericQuestion.tsx similarity index 73% rename from web/src/components/questions/GenericQuestion.jsx rename to web/src/components/questions/GenericQuestion.tsx index 8028b4be97..6a3a8f974f 100644 --- a/web/src/components/questions/GenericQuestion.jsx +++ b/web/src/components/questions/GenericQuestion.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -23,10 +23,23 @@ import React from "react"; import { Text } from "@patternfly/react-core"; import { Popup } from "~/components/core"; import { QuestionActions } from "~/components/questions"; +import { AnswerCallback, Question } from "~/types/questions"; import { _ } from "~/i18n"; -export default function GenericQuestion({ question, answerCallback }) { - const actionCallback = (option) => { +/** + * Component for rendering generic questions + * + * @param question - the question to be answered + * @param answerCallback - the callback to be triggered on answer + */ +export default function GenericQuestion({ + question, + answerCallback, +}: { + question: Question; + answerCallback: AnswerCallback; +}): React.ReactNode { + const actionCallback = (option: string) => { question.answer = option; answerCallback(question); }; diff --git a/web/src/components/questions/LuksActivationQuestion.test.jsx b/web/src/components/questions/LuksActivationQuestion.test.tsx similarity index 77% rename from web/src/components/questions/LuksActivationQuestion.test.jsx rename to web/src/components/questions/LuksActivationQuestion.test.tsx index 8c56815873..41a5326d05 100644 --- a/web/src/components/questions/LuksActivationQuestion.test.jsx +++ b/web/src/components/questions/LuksActivationQuestion.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -23,23 +23,25 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { LuksActivationQuestion } from "~/components/questions"; - -let question; -const answerFn = jest.fn(); +import { AnswerCallback, Question } from "~/types/questions"; + +let question: Question; +const questionMock: Question = { + id: 1, + class: "storage.luks_activation", + text: "A Luks device found. Do you want to open it?", + options: ["decrypt", "skip"], + defaultOption: "decrypt", + data: { attempt: "1" }, +}; +const answerFn: AnswerCallback = jest.fn(); const renderQuestion = () => plainRender(); describe("LuksActivationQuestion", () => { beforeEach(() => { - question = { - id: 1, - class: "storage.luks_activation", - text: "A Luks device found. Do you want to open it?", - options: ["decrypt", "skip"], - defaultOption: "decrypt", - data: { attempt: "1" }, - }; + question = { ...questionMock }; }); it("renders the question text", async () => { @@ -48,13 +50,6 @@ describe("LuksActivationQuestion", () => { await screen.findByText(question.text); }); - it("contains a textinput for entering the password", async () => { - renderQuestion(); - - const passwordInput = await screen.findByLabelText("Encryption Password"); - expect(passwordInput).not.toBeNull(); - }); - describe("when it is the first attempt", () => { it("does not contain a warning", async () => { renderQuestion(); @@ -66,14 +61,7 @@ describe("LuksActivationQuestion", () => { describe("when it is not the first attempt", () => { beforeEach(() => { - question = { - id: 1, - class: "storage.luks_activation", - text: "A Luks device found. Do you want to open it?", - options: ["decrypt", "skip"], - defaultOption: "decrypt", - data: { attempt: "3" }, - }; + question = { ...questionMock, data: { attempt: "2" } }; }); it("contains a warning", async () => { @@ -84,17 +72,6 @@ describe("LuksActivationQuestion", () => { }); describe("when the user selects one of the options", () => { - beforeEach(() => { - question = { - id: 1, - class: "storage.luks_activation", - text: "A Luks device found. Do you want to open it?", - options: ["decrypt", "skip"], - defaultOption: "decrypt", - data: { attempt: "1" }, - }; - }); - describe("by clicking on 'Skip'", () => { it("calls the callback after setting both, answer and password", async () => { const { user } = renderQuestion(); diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.tsx similarity index 76% rename from web/src/components/questions/LuksActivationQuestion.jsx rename to web/src/components/questions/LuksActivationQuestion.tsx index 8e31b915fc..233a1367f7 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -20,27 +20,41 @@ */ import React, { useState } from "react"; -import { Alert, Form, FormGroup, Text } from "@patternfly/react-core"; +import { Alert as PFAlert, Form, FormGroup, Text } from "@patternfly/react-core"; import { Icon } from "~/components/layout"; import { PasswordInput, Popup } from "~/components/core"; import { QuestionActions } from "~/components/questions"; import { _ } from "~/i18n"; -const renderAlert = (attempt) => { - if (!attempt || attempt === 1) return null; +/** + * Internal component for rendering an alert if given password failed + */ +const Alert = ({ attempt }: { attempt: string | undefined }): React.ReactNode => { + if (!attempt || parseInt(attempt) === 1) return null; return ( // TRANSLATORS: error message, user entered a wrong password - + ); }; +/** + * Component for rendering questions related to LUKS activation + * + * @param question - the question to be answered + * @param answerCallback - the callback to be triggered on answer + */ export default function LuksActivationQuestion({ question, answerCallback }) { const [password, setPassword] = useState(question.password || ""); const conditions = { disable: { decrypt: password === "" } }; const defaultAction = "decrypt"; - const actionCallback = (option) => { + const actionCallback = (option: string) => { question.password = password; question.answer = option; answerCallback(question); @@ -60,7 +74,7 @@ export default function LuksActivationQuestion({ question, answerCallback }) { aria-label={_("Question")} titleIconVariant={() => } > - {renderAlert(parseInt(question.data.attempt))} + {question.text}
{/* TRANSLATORS: field label */} diff --git a/web/src/components/questions/QuestionActions.test.jsx b/web/src/components/questions/QuestionActions.test.tsx similarity index 97% rename from web/src/components/questions/QuestionActions.test.jsx rename to web/src/components/questions/QuestionActions.test.tsx index f390d1d2d2..92276120f5 100644 --- a/web/src/components/questions/QuestionActions.test.jsx +++ b/web/src/components/questions/QuestionActions.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -23,10 +23,11 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import { QuestionActions } from "~/components/questions"; +import { Question } from "~/types/questions"; let defaultOption = "sure"; -let question = { +let question: Question = { id: 1, text: "Should we use a component for rendering actions?", options: ["no", "maybe", "sure"], diff --git a/web/src/components/questions/QuestionActions.jsx b/web/src/components/questions/QuestionActions.tsx similarity index 74% rename from web/src/components/questions/QuestionActions.jsx rename to web/src/components/questions/QuestionActions.tsx index f0ececc376..c90539fd20 100644 --- a/web/src/components/questions/QuestionActions.jsx +++ b/web/src/components/questions/QuestionActions.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -27,11 +27,8 @@ import { Popup } from "~/components/core"; * Returns given text capitalized * * TODO: make it work with i18n - * - * @param {String} text - string to be capitalized - * @return {String} capitalized text */ -const label = (text) => `${text[0].toUpperCase()}${text.slice(1)}`; +const label = (text: string): string => `${text[0].toUpperCase()}${text.slice(1)}`; /** * A component for building a Question actions, using the defaultAction @@ -42,13 +39,23 @@ const label = (text) => `${text[0].toUpperCase()}${text.slice(1)}`; * React.Fragment (aka <>) here for wrapping the actions instead of directly using the Popup.Actions. * * @param {object} props - component props - * @param {Array.} props.actions - the actions to be shown - * @param {String} [props.defaultAction] - the action to be shown as primary - * @param {function} props.actionCallback - the function to be called when user clicks on action - * @param {Object} [props.conditions={}] - an object holding conditions, like when an action is disabled + * @param props.actions - the actions to be shown + * @param props.defaultAction - the action to be shown as primary + * @param props.actionCallback - the function to be called when user clicks on action + * @param props.conditions={} - an object holding conditions, like when an action is disabled */ -export default function QuestionActions({ actions, defaultAction, actionCallback, conditions }) { - let [[primaryAction], secondaryActions] = partition(actions, (a) => a === defaultAction); +export default function QuestionActions({ + actions, + defaultAction, + actionCallback, + conditions = {}, +}: { + actions: string[]; + defaultAction?: string; + actionCallback: (action: string) => void; + conditions?: { disable?: { [key: string]: boolean } }; +}): React.ReactNode { + let [[primaryAction], secondaryActions] = partition(actions, (a: string) => a === defaultAction); // Ensure there is always a primary action if (!primaryAction) [primaryAction, ...secondaryActions] = secondaryActions; diff --git a/web/src/components/questions/QuestionWithPassword.test.jsx b/web/src/components/questions/QuestionWithPassword.test.tsx similarity index 87% rename from web/src/components/questions/QuestionWithPassword.test.jsx rename to web/src/components/questions/QuestionWithPassword.test.tsx index bedcbb3cb1..bf5b3d0c3d 100644 --- a/web/src/components/questions/QuestionWithPassword.test.jsx +++ b/web/src/components/questions/QuestionWithPassword.test.tsx @@ -23,25 +23,21 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { QuestionWithPassword } from "~/components/questions"; +import { Question } from "~/types/questions"; -let question; const answerFn = jest.fn(); +const question: Question = { + id: 1, + class: "question.password", + text: "Random question. Will you provide random password?", + options: ["ok", "cancel"], + defaultOption: "cancel", +}; const renderQuestion = () => plainRender(); describe("QuestionWithPassword", () => { - beforeEach(() => { - question = { - id: 1, - class: "question.password", - text: "Random question. Will you provide random password?", - options: ["ok", "cancel"], - defaultOption: "cancel", - data: {}, - }; - }); - it("renders the question text", () => { renderQuestion(); diff --git a/web/src/components/questions/QuestionWithPassword.jsx b/web/src/components/questions/QuestionWithPassword.tsx similarity index 78% rename from web/src/components/questions/QuestionWithPassword.jsx rename to web/src/components/questions/QuestionWithPassword.tsx index 14e534ec44..651c875c85 100644 --- a/web/src/components/questions/QuestionWithPassword.jsx +++ b/web/src/components/questions/QuestionWithPassword.tsx @@ -20,17 +20,30 @@ */ import React, { useState } from "react"; -import { Alert, Form, FormGroup, Text } from "@patternfly/react-core"; +import { Form, FormGroup, Text } from "@patternfly/react-core"; import { Icon } from "~/components/layout"; import { PasswordInput, Popup } from "~/components/core"; import { QuestionActions } from "~/components/questions"; +import { AnswerCallback, Question } from "~/types/questions"; import { _ } from "~/i18n"; -export default function QuestionWithPassword({ question, answerCallback }) { +/** + * Component for rendering questions asking for password + * + * @param question - the question to be answered + * @param answerCallback - the callback to be triggered on answer + */ +export default function QuestionWithPassword({ + question, + answerCallback, +}: { + question: Question; + answerCallback: AnswerCallback; +}): React.ReactNode { const [password, setPassword] = useState(question.password || ""); const defaultAction = question.defaultOption; - const actionCallback = (option) => { + const actionCallback = (option: string) => { question.password = password; question.answer = option; answerCallback(question); diff --git a/web/src/components/questions/Questions.test.jsx b/web/src/components/questions/Questions.test.tsx similarity index 91% rename from web/src/components/questions/Questions.test.jsx rename to web/src/components/questions/Questions.test.tsx index 02a2459d58..a68662dcc3 100644 --- a/web/src/components/questions/Questions.test.jsx +++ b/web/src/components/questions/Questions.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -23,10 +23,10 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender, plainRender } from "~/test-utils"; import { Questions } from "~/components/questions"; -import { QuestionType } from "~/types/questions"; +import { Question, QuestionType } from "~/types/questions"; import * as GenericQuestionComponent from "~/components/questions/GenericQuestion"; -let mockQuestions; +let mockQuestions: Question[]; const mockMutation = jest.fn(); jest.mock("~/components/questions/LuksActivationQuestion", () => () => ( @@ -43,15 +43,15 @@ jest.mock("~/queries/questions", () => ({ useQuestionsConfig: () => ({ mutate: mockMutation }), })); -const genericQuestion = { +const genericQuestion: Question = { id: 1, type: QuestionType.generic, text: "Do you write unit tests?", options: ["always", "sometimes", "never"], defaultOption: "sometimes", }; -const passwordQuestion = { id: 1, type: QuestionType.withPassword }; -const luksActivationQuestion = { id: 1, class: "storage.luks_activation" }; +const passwordQuestion: Question = { id: 1, type: QuestionType.withPassword }; +const luksActivationQuestion: Question = { id: 2, class: "storage.luks_activation" }; describe("Questions", () => { afterEach(() => { diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.tsx similarity index 88% rename from web/src/components/questions/Questions.jsx rename to web/src/components/questions/Questions.tsx index 1da02c97a6..e08d31d60c 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -19,8 +19,6 @@ * find current contact information at www.suse.com. */ -// @ts-check - import React from "react"; import { GenericQuestion, @@ -28,16 +26,16 @@ import { LuksActivationQuestion, } from "~/components/questions"; import { useQuestions, useQuestionsConfig, useQuestionsChanges } from "~/queries/questions"; -import { Question, QuestionType } from "~/types/questions"; +import { AnswerCallback, QuestionType } from "~/types/questions"; -export default function Questions() { +export default function Questions(): React.ReactNode { useQuestionsChanges(); const pendingQuestions = useQuestions(); const questionsConfig = useQuestionsConfig(); if (pendingQuestions.length === 0) return null; - const answerQuestion = (/** @type {Question} */ answeredQuestion) => + const answerQuestion: AnswerCallback = (answeredQuestion) => questionsConfig.mutate(answeredQuestion); // Renders the first pending question diff --git a/web/src/components/questions/index.js b/web/src/components/questions/index.ts similarity index 96% rename from web/src/components/questions/index.js rename to web/src/components/questions/index.ts index 15cc7dee28..0633e0610c 100644 --- a/web/src/components/questions/index.js +++ b/web/src/components/questions/index.ts @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * From 7c7b049b9ba2f2b5a25817e1f350506ad03ae207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 31 Jul 2024 14:10:20 +0100 Subject: [PATCH 06/10] refactor(web): stop exporting everything from components/questions --- web/src/components/questions/GenericQuestion.test.tsx | 2 +- web/src/components/questions/GenericQuestion.tsx | 2 +- .../components/questions/LuksActivationQuestion.test.tsx | 2 +- web/src/components/questions/LuksActivationQuestion.tsx | 2 +- web/src/components/questions/QuestionActions.test.tsx | 2 +- .../components/questions/QuestionWithPassword.test.tsx | 2 +- web/src/components/questions/QuestionWithPassword.tsx | 2 +- web/src/components/questions/Questions.test.tsx | 2 +- web/src/components/questions/Questions.tsx | 8 +++----- web/src/components/questions/index.ts | 4 ---- 10 files changed, 11 insertions(+), 17 deletions(-) diff --git a/web/src/components/questions/GenericQuestion.test.tsx b/web/src/components/questions/GenericQuestion.test.tsx index c5f3536688..a45855a49b 100644 --- a/web/src/components/questions/GenericQuestion.test.tsx +++ b/web/src/components/questions/GenericQuestion.test.tsx @@ -22,8 +22,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import { GenericQuestion } from "~/components/questions"; import { Question } from "~/types/questions"; +import GenericQuestion from "~/components/questions/GenericQuestion"; const question: Question = { id: 1, diff --git a/web/src/components/questions/GenericQuestion.tsx b/web/src/components/questions/GenericQuestion.tsx index 6a3a8f974f..d1f2f34de9 100644 --- a/web/src/components/questions/GenericQuestion.tsx +++ b/web/src/components/questions/GenericQuestion.tsx @@ -22,8 +22,8 @@ import React from "react"; import { Text } from "@patternfly/react-core"; import { Popup } from "~/components/core"; -import { QuestionActions } from "~/components/questions"; import { AnswerCallback, Question } from "~/types/questions"; +import QuestionActions from "~/components/questions/QuestionActions"; import { _ } from "~/i18n"; /** diff --git a/web/src/components/questions/LuksActivationQuestion.test.tsx b/web/src/components/questions/LuksActivationQuestion.test.tsx index 41a5326d05..c2da7b99cd 100644 --- a/web/src/components/questions/LuksActivationQuestion.test.tsx +++ b/web/src/components/questions/LuksActivationQuestion.test.tsx @@ -22,8 +22,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import { LuksActivationQuestion } from "~/components/questions"; import { AnswerCallback, Question } from "~/types/questions"; +import LuksActivationQuestion from "~/components/questions/LuksActivationQuestion"; let question: Question; const questionMock: Question = { diff --git a/web/src/components/questions/LuksActivationQuestion.tsx b/web/src/components/questions/LuksActivationQuestion.tsx index 233a1367f7..8862defbc6 100644 --- a/web/src/components/questions/LuksActivationQuestion.tsx +++ b/web/src/components/questions/LuksActivationQuestion.tsx @@ -23,7 +23,7 @@ import React, { useState } from "react"; import { Alert as PFAlert, Form, FormGroup, Text } from "@patternfly/react-core"; import { Icon } from "~/components/layout"; import { PasswordInput, Popup } from "~/components/core"; -import { QuestionActions } from "~/components/questions"; +import QuestionActions from "~/components/questions/QuestionActions"; import { _ } from "~/i18n"; /** diff --git a/web/src/components/questions/QuestionActions.test.tsx b/web/src/components/questions/QuestionActions.test.tsx index 92276120f5..ad6fc6280c 100644 --- a/web/src/components/questions/QuestionActions.test.tsx +++ b/web/src/components/questions/QuestionActions.test.tsx @@ -22,8 +22,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; -import { QuestionActions } from "~/components/questions"; import { Question } from "~/types/questions"; +import QuestionActions from "~/components/questions/QuestionActions"; let defaultOption = "sure"; diff --git a/web/src/components/questions/QuestionWithPassword.test.tsx b/web/src/components/questions/QuestionWithPassword.test.tsx index bf5b3d0c3d..371daaf20a 100644 --- a/web/src/components/questions/QuestionWithPassword.test.tsx +++ b/web/src/components/questions/QuestionWithPassword.test.tsx @@ -22,8 +22,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import { QuestionWithPassword } from "~/components/questions"; import { Question } from "~/types/questions"; +import QuestionWithPassword from "~/components/questions/QuestionWithPassword"; const answerFn = jest.fn(); const question: Question = { diff --git a/web/src/components/questions/QuestionWithPassword.tsx b/web/src/components/questions/QuestionWithPassword.tsx index 651c875c85..f18f250625 100644 --- a/web/src/components/questions/QuestionWithPassword.tsx +++ b/web/src/components/questions/QuestionWithPassword.tsx @@ -23,8 +23,8 @@ import React, { useState } from "react"; import { Form, FormGroup, Text } from "@patternfly/react-core"; import { Icon } from "~/components/layout"; import { PasswordInput, Popup } from "~/components/core"; -import { QuestionActions } from "~/components/questions"; import { AnswerCallback, Question } from "~/types/questions"; +import QuestionActions from "~/components/questions/QuestionActions"; import { _ } from "~/i18n"; /** diff --git a/web/src/components/questions/Questions.test.tsx b/web/src/components/questions/Questions.test.tsx index a68662dcc3..f85555e760 100644 --- a/web/src/components/questions/Questions.test.tsx +++ b/web/src/components/questions/Questions.test.tsx @@ -22,8 +22,8 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender, plainRender } from "~/test-utils"; -import { Questions } from "~/components/questions"; import { Question, QuestionType } from "~/types/questions"; +import Questions from "~/components/questions/Questions"; import * as GenericQuestionComponent from "~/components/questions/GenericQuestion"; let mockQuestions: Question[]; diff --git a/web/src/components/questions/Questions.tsx b/web/src/components/questions/Questions.tsx index e08d31d60c..42ef731d26 100644 --- a/web/src/components/questions/Questions.tsx +++ b/web/src/components/questions/Questions.tsx @@ -20,11 +20,9 @@ */ import React from "react"; -import { - GenericQuestion, - QuestionWithPassword, - LuksActivationQuestion, -} from "~/components/questions"; +import GenericQuestion from "~/components/questions/GenericQuestion"; +import QuestionWithPassword from "~/components/questions/QuestionWithPassword"; +import LuksActivationQuestion from "~/components/questions/LuksActivationQuestion"; import { useQuestions, useQuestionsConfig, useQuestionsChanges } from "~/queries/questions"; import { AnswerCallback, QuestionType } from "~/types/questions"; diff --git a/web/src/components/questions/index.ts b/web/src/components/questions/index.ts index 0633e0610c..95dcc90e28 100644 --- a/web/src/components/questions/index.ts +++ b/web/src/components/questions/index.ts @@ -19,8 +19,4 @@ * find current contact information at www.suse.com. */ -export { default as QuestionActions } from "./QuestionActions"; -export { default as GenericQuestion } from "./GenericQuestion"; -export { default as QuestionWithPassword } from "./QuestionWithPassword"; -export { default as LuksActivationQuestion } from "./LuksActivationQuestion"; export { default as Questions } from "./Questions"; From 459f06ca4c5727711bde7c64cfa7775a5e161e44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= <1691872+dgdavid@users.noreply.github.com> Date: Fri, 2 Aug 2024 12:06:46 +0100 Subject: [PATCH 07/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Imobach González Sosa --- .../components/questions/LuksActivationQuestion.test.tsx | 4 ++-- web/src/components/questions/LuksActivationQuestion.tsx | 9 ++------- web/src/components/questions/QuestionActions.tsx | 6 +++--- web/src/components/questions/Questions.test.tsx | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/web/src/components/questions/LuksActivationQuestion.test.tsx b/web/src/components/questions/LuksActivationQuestion.test.tsx index c2da7b99cd..3af25fc170 100644 --- a/web/src/components/questions/LuksActivationQuestion.test.tsx +++ b/web/src/components/questions/LuksActivationQuestion.test.tsx @@ -54,7 +54,7 @@ describe("LuksActivationQuestion", () => { it("does not contain a warning", async () => { renderQuestion(); - const warning = screen.queryByText(/Given encryption password/); + const warning = screen.queryByText("The encryption password did not work"); expect(warning).toBeNull(); }); }); @@ -67,7 +67,7 @@ describe("LuksActivationQuestion", () => { it("contains a warning", async () => { renderQuestion(); - await screen.findByText(/Given encryption password/); + await screen.findByText("The encryption password did not work"); }); }); diff --git a/web/src/components/questions/LuksActivationQuestion.tsx b/web/src/components/questions/LuksActivationQuestion.tsx index 8862defbc6..5998f27472 100644 --- a/web/src/components/questions/LuksActivationQuestion.tsx +++ b/web/src/components/questions/LuksActivationQuestion.tsx @@ -29,17 +29,12 @@ import { _ } from "~/i18n"; /** * Internal component for rendering an alert if given password failed */ -const Alert = ({ attempt }: { attempt: string | undefined }): React.ReactNode => { +const Alert = ({ attempt }: { attempt?: string }): React.ReactNode => { if (!attempt || parseInt(attempt) === 1) return null; return ( // TRANSLATORS: error message, user entered a wrong password - + ); }; diff --git a/web/src/components/questions/QuestionActions.tsx b/web/src/components/questions/QuestionActions.tsx index c90539fd20..c96ddeb294 100644 --- a/web/src/components/questions/QuestionActions.tsx +++ b/web/src/components/questions/QuestionActions.tsx @@ -39,9 +39,9 @@ const label = (text: string): string => `${text[0].toUpperCase()}${text.slice(1) * React.Fragment (aka <>) here for wrapping the actions instead of directly using the Popup.Actions. * * @param {object} props - component props - * @param props.actions - the actions to be shown - * @param props.defaultAction - the action to be shown as primary - * @param props.actionCallback - the function to be called when user clicks on action + * @param props.actions - the actions show + * @param props.defaultAction - the action to show as primary + * @param props.actionCallback - the function to call when the user clicks on the action * @param props.conditions={} - an object holding conditions, like when an action is disabled */ export default function QuestionActions({ diff --git a/web/src/components/questions/Questions.test.tsx b/web/src/components/questions/Questions.test.tsx index f85555e760..31ce42bb9b 100644 --- a/web/src/components/questions/Questions.test.tsx +++ b/web/src/components/questions/Questions.test.tsx @@ -74,7 +74,7 @@ describe("Questions", () => { mockQuestions = [genericQuestion]; }); - it("triggers the useQuestionMutationk", async () => { + it("triggers the useQuestionMutation", async () => { const { user } = plainRender(); const button = screen.getByRole("button", { name: "Always" }); await user.click(button); From a4ffb98d0b50d53d3f1192f3336cd36528d1e9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 2 Aug 2024 13:13:51 +0100 Subject: [PATCH 08/10] fix(web): use the right http verb for mutate a question PUT instead of PATCH. --- web/src/queries/questions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/queries/questions.ts b/web/src/queries/questions.ts index 118fd0e029..62d505b15d 100644 --- a/web/src/queries/questions.ts +++ b/web/src/queries/questions.ts @@ -79,7 +79,7 @@ const useQuestionsConfig = () => { } return fetch(`/api/questions/${question.id}/answer`, { - method: "PATCH", + method: "PUT", body: JSON.stringify(answer), headers: { "Content-Type": "application/json", From 59d1a5714ab3102fc37261b32483496a6e262662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 2 Aug 2024 13:22:09 +0100 Subject: [PATCH 09/10] fix(web): improve buildQuestion method --- web/src/queries/questions.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/web/src/queries/questions.ts b/web/src/queries/questions.ts index 62d505b15d..f5b56f41b9 100644 --- a/web/src/queries/questions.ts +++ b/web/src/queries/questions.ts @@ -35,22 +35,16 @@ type APIQuestion = { * TODO: improve/simplify it once the backend API is improved. */ function buildQuestion(httpQuestion: APIQuestion) { - let question: Question; + const question: Question = { ...httpQuestion.generic }; if (httpQuestion.generic) { - question = { - ...httpQuestion.generic, - type: QuestionType.generic, - answer: httpQuestion.generic.answer, - }; + question.type = QuestionType.generic; + question.answer = httpQuestion.generic.answer; } if (httpQuestion.withPassword) { - question = { - id: httpQuestion.generic.id, - type: QuestionType.withPassword, - password: httpQuestion.withPassword.password, - }; + question.type = QuestionType.withPassword; + question.password = httpQuestion.withPassword.password; } return question; From 0f03be9d1691e19f5e3004647b6e9b8d7416f08a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 2 Aug 2024 13:45:52 +0100 Subject: [PATCH 10/10] fix(web): improve the questions presentation By using PF/Stack#hasGutter for adding vertical space between internal components. --- .../questions/LuksActivationQuestion.tsx | 30 ++++++++++--------- .../questions/QuestionWithPassword.tsx | 28 +++++++++-------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/web/src/components/questions/LuksActivationQuestion.tsx b/web/src/components/questions/LuksActivationQuestion.tsx index 5998f27472..9b666ddd18 100644 --- a/web/src/components/questions/LuksActivationQuestion.tsx +++ b/web/src/components/questions/LuksActivationQuestion.tsx @@ -20,7 +20,7 @@ */ import React, { useState } from "react"; -import { Alert as PFAlert, Form, FormGroup, Text } from "@patternfly/react-core"; +import { Alert as PFAlert, Form, FormGroup, Text, Stack } from "@patternfly/react-core"; import { Icon } from "~/components/layout"; import { PasswordInput, Popup } from "~/components/core"; import QuestionActions from "~/components/questions/QuestionActions"; @@ -69,19 +69,21 @@ export default function LuksActivationQuestion({ question, answerCallback }) { aria-label={_("Question")} titleIconVariant={() => } > - - {question.text} - - {/* TRANSLATORS: field label */} - - setPassword(value)} - /> - - + + + {question.text} +
+ {/* TRANSLATORS: field label */} + + setPassword(value)} + /> + +
+
} > - {question.text} -
- {/* TRANSLATORS: field label */} - - setPassword(value)} - /> - -
+ + {question.text} +
+ {/* TRANSLATORS: field label */} + + setPassword(value)} + /> + +
+