Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ - refactor: migrate detail process review to BaseListView #399

Merged
merged 5 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ async def test_scenario_record_manager_process_review(self):
await self.when.user_clicks_button(page, "Destruction list to process")
await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000")

# TODO
await self.when.user_clicks_checkbox(page, "(de)selecteer rij")
await self.when.user_clicks_button(page, "Muteren")

# Fill selectielijstklasse as it's probably missing.
await self.when.user_clicks_radio(page, "Aanpassen van selectielijstklasse")
Expand All @@ -66,7 +65,7 @@ async def test_scenario_record_manager_process_review(self):
await self.when.user_fills_form_field(page, "Reden", "Andere datum")
await self.when.user_clicks_button(page, "muteren")
await self.when.user_clicks_button(page, "Opnieuw indienen")
await self.when.user_fills_form_field(page, "Opmerking", "Datum aangepast")
await self.when.user_fills_form_field(page, "Opmerking", "Datum aangepast", None, 1)
await self.when.user_clicks_button(page, "Opnieuw indienen", 1)
await self.then.path_should_be(page, "/destruction-lists")

Expand Down
28 changes: 17 additions & 11 deletions backend/src/openarchiefbeheer/utils/tests/gherkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,25 +344,31 @@ async def _user_clicks(self, role, page, name, index=0):
await element.wait_for()
await element.click()

async def user_fills_form_field(self, page, label, value, role=None):
select = await page.query_selector(f'.mykn-select[title="{label}"]')
if select: # has content so select?
await select.click()
options = await select.query_selector_all(".mykn-option")
async def user_fills_form_field(self, page, label, value, role=None, index=0):
selects = await page.query_selector_all(f'.mykn-select[title="{label}"]')
try:
select = selects[index]

if select: # has content so select?
await select.click()
options = await select.query_selector_all(".mykn-option")

for option in options:
text_content = await option.text_content()
if text_content == value:
return await option.click()
for option in options:
text_content = await option.text_content()
if text_content == value:
return await option.click()

return
return
except IndexError:
pass

if role:
locator = page.get_by_label(label).and_(page.get_by_role("textbox"))
else:
locator = page.get_by_label(label)

await locator.fill(value)
elements = await locator.all()
await elements[index].fill(value)

async def user_filters_zaken(self, page, name, value):
locator = page.get_by_role("textbox", name=name)
Expand Down
14 changes: 0 additions & 14 deletions frontend/.storybook/playFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,20 +279,6 @@ export const fillButtonConfirmationForm: PlayFunction<ReactRenderer> = async (
});
};

/**
* Clicks element at position `elementIndex`, within <tbody> if `inTbody` is truthy.
* Then fills in dialog form, submits if `submitForm` is truthy.
* @param context
*/
export const fillCheckboxConfirmationForm: PlayFunction<ReactRenderer> = async (
context,
) => {
await fillConfirmationForm({
...context,
parameters: { ...context.parameters, checked: true, role: "checkbox" },
});
};

/**
* Clicks element at position `elementIndex`, within <tbody> if `inTbody` is truthy.
* Then fills in dialog form, submits if `submitForm` is truthy.
Expand Down
47 changes: 31 additions & 16 deletions frontend/src/lib/zaakSelection/zaakSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,6 @@ export async function setAllZakenSelected(key: string, selected: boolean) {
}
}

/**
* Gets the zaak selection.
* Note: only the `url` of selected `zaken` are stored.
* Note: This function is async to accommodate possible future refactors.
* @param key A key identifying the selection
*/
export async function getZaakSelection<DetailType = unknown>(key: string) {
const computedKey = _getComputedKey(key);
const json = sessionStorage.getItem(computedKey) || "{}";
return JSON.parse(json) as ZaakSelection<DetailType>;
}

/**
* Gets the zaak selection, applies a filter to the detail object.
* Note: only the `url` of selected `zaken` are stored.
Expand All @@ -97,7 +85,7 @@ export async function getFilteredZaakSelection<DetailType = unknown>(
exp?: Partial<DetailType>,
selectedOnly = true,
) {
const selection = await getZaakSelection<DetailType>(key);
const selection = await _getZaakSelection<DetailType>(key);
const entries = Object.entries(selection);

const filteredEntries = entries.filter(([url, { selected, detail }]) => {
Expand Down Expand Up @@ -143,7 +131,7 @@ export async function getZaakSelectionItem<DetailType = unknown>(
zaak: string | Zaak,
selectedOnly = true,
) {
const zaakSelection = await getZaakSelection<DetailType>(key);
const zaakSelection = await _getZaakSelection<DetailType>(key);
const url = _getZaakUrl(zaak);
return zaakSelection[url]?.selected || !selectedOnly
? zaakSelection[url]
Expand Down Expand Up @@ -187,7 +175,7 @@ export async function isZaakSelected<DetailType = unknown>(
key: string,
zaak: string | Zaak,
) {
const zaakSelection = await getZaakSelection<DetailType>(key);
const zaakSelection = await _getZaakSelection<DetailType>(key);
const url = _getZaakUrl(zaak);
return zaakSelection[url]?.selected;
}
Expand Down Expand Up @@ -215,7 +203,7 @@ export async function _mutateZaakSelection<DetailType = unknown>(
}
}

const currentZaakSelection = await getZaakSelection<DetailType>(key);
const currentZaakSelection = await _getZaakSelection<DetailType>(key);
const urls = _getZaakUrls(zaken);

const zaakSelectionOverrides = urls.reduce<ZaakSelection<DetailType>>(
Expand All @@ -237,9 +225,34 @@ export async function _mutateZaakSelection<DetailType = unknown>(
return setZaakSelection(key, combinedZaakSelection);
}

/**
* Gets the zaak selection.
* Note: only the `url` of selected `zaken` are stored.
* Note: This function is async to accommodate possible future refactors.
* @param key A key identifying the selection
* @private
*/
async function _getZaakSelection<DetailType = unknown>(key: string) {
const computedKey = _getComputedKey(key);
const json = sessionStorage.getItem(computedKey) || "{}";
return JSON.parse(json) as ZaakSelection<DetailType>;
}

/**
* @deprecated public use outside `zaakSelection.ts` is deprecated due to performance concerns.
*/
export async function getZaakSelection<DetailType = unknown>(key: string) {
if (process.env.NODE_ENV === "development") {
console.warn("public use of _getZaakSelection is deprecated.");
}

return _getZaakSelection<DetailType>(key);
}

/**
* Computes the prefixed cache key.
* @param key A key identifying the selection
* @private
*/
function _getComputedKey(key: string): string {
return `oab.lib.zaakSelection.${key}`;
Expand All @@ -248,6 +261,7 @@ function _getComputedKey(key: string): string {
/**
* Returns the urls based on an `Array` of `string`s or `Zaak` objects.
* @param zaken An array containing either `Zaak.url` or `Zaak` objects
* @private
*/
function _getZaakUrls(zaken: Array<string | Zaak>) {
return zaken.map(_getZaakUrl);
Expand All @@ -256,6 +270,7 @@ function _getZaakUrls(zaken: Array<string | Zaak>) {
/**
* Returns the url based on a `string` or `Zaak` object.
* @param zaak Either a `Zaak.url` or `Zaak` object.
* @private
*/
function _getZaakUrl(zaak: string | Zaak) {
return isPrimitive(zaak) ? zaak : (zaak.url as string);
Expand Down
17 changes: 14 additions & 3 deletions frontend/src/pages/destructionlist/abstract/BaseListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ListTemplateProps,
Solid,
TypedField,
formatMessage,
} from "@maykin-ui/admin-ui";
import React, { useCallback, useMemo } from "react";
import { useNavigation } from "react-router-dom";
Expand All @@ -25,6 +26,9 @@ import {
useZaakSelection,
} from "../hooks/useZaakSelection";

/** The template used to format urls to an external application providing zaak details. */
const REACT_APP_ZAAK_URL_TEMPLATE = process.env.REACT_APP_ZAAK_URL_TEMPLATE;

export type BaseListViewProps = React.PropsWithChildren<{
storageKey: string;
title?: string;
Expand All @@ -35,7 +39,8 @@ export type BaseListViewProps = React.PropsWithChildren<{
paginatedZaken: PaginatedZaken;
secondaryNavigationItems?: ListTemplateProps["secondaryNavigationItems"];

selectable?: boolean;
// Visible means that no checkboxes appear, but the zaken are marked if selected (via another route).
selectable?: boolean | "visible";
allowSelectAllPages?: boolean;
selectionActions?: ButtonProps[];
initiallySelectedZakenOnPage?: Zaak[];
Expand Down Expand Up @@ -79,6 +84,12 @@ export function BaseListView({
const [, setFilterField] = useFilter();
const [sort, setSort] = useSort();

// Object list.
const objectList = paginatedZaken.results.map((zaak) => ({
...zaak,
href: formatMessage(REACT_APP_ZAAK_URL_TEMPLATE || "", zaak),
}));

// Fields.
const [fields, setFields, filterTransform] = useFields(
destructionList,
Expand Down Expand Up @@ -164,7 +175,7 @@ export function BaseListView({
fieldsSelectable: true,
pageSize: 100,
showPaginator: true,
selectable: selectable,
selectable: selectable === true,
filterable: true,
tableLayout: "fixed",

Expand All @@ -175,7 +186,7 @@ export function BaseListView({
fields,
filterTransform,
loading: state === "loading",
objectList: paginatedZaken.results as unknown as AttributeData[],
objectList: objectList,
page,
sort: sort,
selected: selectable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import { ReactRouterDecorator } from "../../../../.storybook/decorators";
import {
assertColumnSelection,
clickButton,
clickElement,
fillButtonConfirmationForm,
fillCheckboxConfirmationForm,
fillForm,
} from "../../../../.storybook/playFunctions";
import { auditLogFactory } from "../../../fixtures/auditLog";
Expand All @@ -25,7 +23,7 @@ import {
FIXTURE_SELECTIELIJSTKLASSE_CHOICES_MAP,
selectieLijstKlasseFactory,
} from "../../../fixtures/selectieLijstKlasseChoices";
import { usersFactory } from "../../../fixtures/user";
import { userFactory, usersFactory } from "../../../fixtures/user";
import {
clearZaakSelection,
getZaakSelection,
Expand Down Expand Up @@ -83,6 +81,12 @@ const meta: Meta<typeof DestructionListDetailPage> = {
status: 200,
response: FIXTURE_SELECTIELIJSTKLASSE_CHOICES,
},
{
url: "http://localhost:8000/api/v1/whoami/?",
method: "GET",
status: 200,
response: userFactory(),
},
],
},
};
Expand Down Expand Up @@ -240,10 +244,11 @@ export const ProcessReview: Story = {
},
},
play: async (context) => {
await fillCheckboxConfirmationForm({
await fillButtonConfirmationForm({
...context,
parameters: {
elementIndex: 0,
name: "Muteren",
formValues: {
"Aanpassen van selectielijstklasse": true,
Selectielijstklasse: selectieLijstKlasseFactory()[0].label,
Expand All @@ -252,10 +257,11 @@ export const ProcessReview: Story = {
},
});

await fillCheckboxConfirmationForm({
await fillButtonConfirmationForm({
...context,
parameters: {
elementIndex: 1,
name: "Muteren",
formValues: {
"Aanpassen van selectielijstklasse": true,
Selectielijstklasse: selectieLijstKlasseFactory()[1].label,
Expand All @@ -264,10 +270,11 @@ export const ProcessReview: Story = {
},
});

await fillCheckboxConfirmationForm({
await fillButtonConfirmationForm({
...context,
parameters: {
elementIndex: 2,
name: "Muteren",
formValues: {
"Aanpassen van selectielijstklasse": true,
Selectielijstklasse: selectieLijstKlasseFactory()[2].label,
Expand Down Expand Up @@ -328,12 +335,10 @@ export const CheckSelectielijstklasseSelection: Story = {
},
},
play: async (context) => {
await clickElement({
await clickButton({
...context,
parameters: {
elementIndex: 0,
role: "checkbox",
checked: true,
name: "Muteren",
},
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,21 @@
import { CardBaseTemplate } from "@maykin-ui/admin-ui";
import React from "react";
import { useLoaderData } from "react-router-dom";

import { DestructionListDetailContext } from "./DestructionListDetail.loader";
import { DestructionListEdit } from "./components/DestructionListEdit/DestructionListEdit";
import { DestructionListProcessReview } from "./components/DestructionListProcessReview/DestructionListProcessReview";
import { DestructionListToolbar } from "./components/DestructionListToolbar/DestructionListToolbar";
import { useSecondaryNavigation } from "./hooks/useSecondaryNavigation";

/**
* Destruction list detail page
*/
export function DestructionListDetailPage() {
const { destructionList } = useLoaderData() as DestructionListDetailContext;
const isInReview = destructionList.status === "changes_requested";
const secondaryNavigationItems = useSecondaryNavigation();

// TODO: SEPARATE ROUTE?
// TODO: SEPARATE ROUTES?
if (!isInReview) {
return <DestructionListEdit />;
} else {
return <DestructionListProcessReview />;
}

// FIXME: MIGRATE TO NEW APPROACH (NEW URL?)
return (
<CardBaseTemplate secondaryNavigationItems={secondaryNavigationItems}>
<DestructionListToolbar />
<DestructionListProcessReview />
</CardBaseTemplate>
);
}
Loading
Loading