ljmotta commented on code in PR #2308:
URL:
https://github.com/apache/incubator-kie-tools/pull/2308#discussion_r1606695002
##########
packages/boxed-expression-component/tests/e2e/boxedExpressions/boxedExpressionHeader.spec.ts:
##########
@@ -22,7 +22,7 @@ import { test, expect } from "../__fixtures__/base";
test.describe("Boxed expression header", () => {
test.beforeEach(async ({ boxedExpressionEditor, browserName }) => {
test.skip(
- browserName !== "chromium",
+ browserName === "webkit",
Review Comment:
👍
##########
packages/boxed-expression-component/tests/e2e/boxedExpressions/filter/populate.spec.ts:
##########
Review Comment:
We left the `populate.spec.ts` to add a complete case. On this file you
could left the creation of `Rebooked Flights` example, or any other example,
like the nested list one.
Add a `filterExpression.spec.ts` to check if it render correctly, you could
move the create a nested filter to this file too and the respective reset/reset
nested. Move the `rename` and `change data type` too.
##########
packages/boxed-expression-component/tests/e2e/boxedExpressions/filter/populate.spec.ts:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import _ from "lodash";
+import { test, expect } from "../../__fixtures__/base";
+
+test.describe("Populate Boxed Filter", () => {
+ test("should rename a filter", async ({ page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByPlaceholder("Expression Name").fill("Filter Expression
Name");
+ await page.keyboard.press("Enter");
+
+ await expect(page.getByRole("columnheader", { name: "Filter Expression
Name (<Undefined>)" })).toBeVisible();
+ });
+
+ test("should change a filter data type", async ({ page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByLabel("<Undefined>").click();
+ await page.getByRole("option", { name: "boolean" }).click();
+ await page.keyboard.press("Enter");
+
+ await expect(page.getByRole("columnheader", { name: "Expression Name
(boolean)" })).toBeVisible();
+ });
+
+ test("should correctly create a Rebooked Flights filter", async ({ stories,
page, boxedExpressionEditor }) => {
+ await stories.openBoxedFilter("base");
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByPlaceholder("Expression Name").fill("Rebooked Flights");
+ await page.getByLabel("<Undefined>").click();
+ await page.getByRole("option", { name: "Any" }).click();
+ await page.keyboard.press("Enter");
+
+ await boxedExpressionEditor.fillFilter({
+ collectionIn: "Passengers",
+ collectionMatch: "item.Flight Number = Flight.Flight Number",
+ });
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-rebooked-flights.png");
+ });
+
+ test("should correctly create a nested filter", async ({
boxedExpressionEditor, page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await boxedExpressionEditor.resetFilter();
+ await boxedExpressionEditor.selectBoxedContext();
+ await boxedExpressionEditor.selectBoxedFilter(page.getByText("Select
expression").first());
+
+ await boxedExpressionEditor.fillFilter({
+ collectionIn: "collection in expression",
+ collectionMatch: "collection match expression",
+ });
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-nested.png");
+ });
+
+ test("should correctly create a filter using list boxed expression", async ({
+ boxedExpressionEditor,
+ page,
+ stories,
+ }) => {
+ await stories.openBoxedFilter("base");
+ await page.getByText("=").first().click({ button: "right" });
+ await page.getByRole("menuitem").getByText("Reset").click();
+ await boxedExpressionEditor.selectBoxedList(page.getByText("Select
expression").first());
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-nested-boxed-list.png");
+ });
+
+ test("should correctly reset a nested filter", async ({
boxedExpressionEditor, page, stories }) => {
+ await stories.openBoxedFilter("nested");
+
+ await boxedExpressionEditor.resetFilter();
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-nested-reset.png");
+ });
+
+ test("should correctly copy and paste a filter - context menu", async ({
+ boxedExpressionEditor,
+ browserName,
+ context,
+ page,
+ stories,
+ }) => {
+ test.skip(
+ browserName === "webkit",
+ "Playwright Webkit doesn't support clipboard permissions:
https://github.com/microsoft/playwright/issues/13037"
+ );
+
+ await context.grantPermissions(["clipboard-read", "clipboard-write"]);
+
+ await stories.openBoxedFilter("rebooked-flights");
+ await boxedExpressionEditor.copyFilter();
+ await boxedExpressionEditor.resetFilter();
+ await boxedExpressionEditor.selectBoxedContext(page.getByText("Select
expression").first());
+ await boxedExpressionEditor.pasteToUndefinedCell(page.getByText("Select
expression").first());
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-copied-and-pasted-as-nested.png");
+ });
Review Comment:
Great addition. Even though we have some generic tests for the `copy/paste`
on the `selection.spec.ts` I guess the suite is lacking more specific tests.
Please move this to the `selection.spec.ts` inside a `Filter Expression`
description block.
##########
packages/boxed-expression-component/tests/e2e/boxedExpressions/filter/populate.spec.ts:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import _ from "lodash";
Review Comment:
I couldn't find the usage
##########
packages/boxed-expression-component/tests/e2e/boxedExpressions/filter/populate.spec.ts:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import _ from "lodash";
+import { test, expect } from "../../__fixtures__/base";
+
+test.describe("Populate Boxed Filter", () => {
+ test("should rename a filter", async ({ page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByPlaceholder("Expression Name").fill("Filter Expression
Name");
+ await page.keyboard.press("Enter");
+
+ await expect(page.getByRole("columnheader", { name: "Filter Expression
Name (<Undefined>)" })).toBeVisible();
+ });
+
+ test("should change a filter data type", async ({ page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByLabel("<Undefined>").click();
+ await page.getByRole("option", { name: "boolean" }).click();
+ await page.keyboard.press("Enter");
+
+ await expect(page.getByRole("columnheader", { name: "Expression Name
(boolean)" })).toBeVisible();
+ });
+
+ test("should correctly create a Rebooked Flights filter", async ({ stories,
page, boxedExpressionEditor }) => {
+ await stories.openBoxedFilter("base");
+ await page.getByRole("columnheader", { name: "Expression Name
(<Undefined>)" }).click();
+ await page.getByPlaceholder("Expression Name").fill("Rebooked Flights");
+ await page.getByLabel("<Undefined>").click();
+ await page.getByRole("option", { name: "Any" }).click();
+ await page.keyboard.press("Enter");
+
+ await boxedExpressionEditor.fillFilter({
+ collectionIn: "Passengers",
+ collectionMatch: "item.Flight Number = Flight.Flight Number",
+ });
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-rebooked-flights.png");
+ });
+
+ test("should correctly create a nested filter", async ({
boxedExpressionEditor, page, stories }) => {
+ await stories.openBoxedFilter("base");
+
+ await boxedExpressionEditor.resetFilter();
+ await boxedExpressionEditor.selectBoxedContext();
+ await boxedExpressionEditor.selectBoxedFilter(page.getByText("Select
expression").first());
+
+ await boxedExpressionEditor.fillFilter({
+ collectionIn: "collection in expression",
+ collectionMatch: "collection match expression",
+ });
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-nested.png");
+ });
+
+ test("should correctly create a filter using list boxed expression", async ({
+ boxedExpressionEditor,
+ page,
+ stories,
+ }) => {
+ await stories.openBoxedFilter("base");
+ await page.getByText("=").first().click({ button: "right" });
+ await page.getByRole("menuitem").getByText("Reset").click();
+ await boxedExpressionEditor.selectBoxedList(page.getByText("Select
expression").first());
+
+ await
expect(boxedExpressionEditor.getContainer()).toHaveScreenshot("boxed-filter-nested-boxed-list.png");
+ });
Review Comment:
Please, add a more complete example here. The list choice looks a little
randomic, as the filter supports any other expression as well.
##########
packages/boxed-expression-component/stories/boxedExpressions/Filter/Filter.stories.tsx:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Meta, StoryObj } from "@storybook/react";
+import { BoxedExpressionEditor, BoxedExpressionEditorProps } from
"../../../src/BoxedExpressionEditor";
+import { BoxedExpressionEditorStory, BoxedExpressionEditorStoryArgs } from
"../../boxedExpressionStoriesWrapper";
+import { generateUuid } from "../../../src/api";
+import { Base as EmptyExpression } from
"../../misc/Empty/EmptyExpression.stories";
+
+// More on how to set up stories at:
https://storybook.js.org/docs/react/writing-stories/introduction#default-export
+const meta: Meta<BoxedExpressionEditorProps> = {
+ title: "Boxed Expressions/Filter",
+ component: BoxedExpressionEditor,
+ includeStories: /^[A-Z]/,
+};
+
+export default meta;
+type Story = StoryObj<BoxedExpressionEditorStoryArgs>;
+
+// More on writing stories with args:
https://storybook.js.org/docs/react/writing-stories/args
+export const Base: Story = {
+ render: (args) => BoxedExpressionEditorStory(),
+ parameters: { exclude: ["dataTypes", "beeGwtService", "pmmlDocuments"] },
+ args: {
+ ...EmptyExpression.args,
+ expression: {
+ __$$element: "filter",
+ "@_id": generateUuid(),
+ "@_label": "Expression Name",
+ in: {
+ "@_id": generateUuid(),
+ expression: {
+ __$$element: "literalExpression",
+ "@_id": generateUuid(),
+ },
+ },
+ match: {
+ "@_id": generateUuid(),
+ expression: {
+ __$$element: "literalExpression",
+ "@_id": generateUuid(),
+ },
Review Comment:
The `base` example should be without expressions.
##########
packages/boxed-expression-component/tests/e2e/__fixtures__/boxedExpression.ts:
##########
@@ -89,6 +94,36 @@ export class BoxedExpressionEditor {
await this.page.getByRole("menuitem", { name: "Function" }).click();
}
+ public async selectBoxedFilter(from: Page | Locator = this.page) {
+ this.select(from);
+ await this.page.getByRole("menuitem", { name: "Filter" }).click();
+ }
+
+ public async copyFilter(from: Page | Locator = this.page) {
+ await from.getByRole("button").getByText("Filter").click();
+ await this.page.getByRole("menuitem").getByText("Copy").click();
+ }
+
+ public async resetFilter(from: Page | Locator = this.page) {
+ await from.getByRole("button").getByText("Filter").click();
+ await this.page.getByRole("menuitem").getByText("Reset").click();
+ }
Review Comment:
Currently, we don't have specific fixtures, which we need to revisit on the
near future. For this case, I guess we could stick with what we have, and use
inline `testId` we already use on some other tests.
```
await page.getByTestId("logic-type-button-test-id").click();
await page.getByRole("menuitem", { name: "copy" }).click();
or
await page.getByRole("menuitem", { name: "reset" }).click();
```
Or we could start creating the specific fixtures. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]