Skip to content

Commit 2a9b7a6

Browse files
authored
Add library removal confirmation to service edit form (PP-2408) (#170)
## Description - Adds support for allowing/disallowing the disassociation of a library from a service integration via the `ServiceEditForm` and its child components. The check is performed synchronously, in real time, when removal is requested via UI interaction. - Adds documentation to help avoid accidental misuse of the `ServiceEditForm.removeLibrary` method. The mechanism separates (1) the function of removing a library association from (2) the function of approving that removal. The default behavior is to allow disassociation. Subclasses of `ServiceEditForm` may override this behavior. ## Motivation and Context This capability enables future real-time verification that a removal is allowed or intended (e.g., via confirmation dialog). [Jira PP-2408] ## How Has This Been Tested? - New and updated tests. - All tests pass locally. - [CI tests](https://github.com/ThePalaceProject/circulation-admin/actions/runs/17103293415) passed. ## Checklist: - [x] - I have updated the documentation accordingly. - [x] All new and existing tests passed.
1 parent 1d03bcf commit 2a9b7a6

File tree

4 files changed

+131
-1
lines changed

4 files changed

+131
-1
lines changed

src/components/ServiceEditForm.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
ProtocolData,
1111
ServiceData,
1212
ServicesData,
13-
SettingData,
1413
} from "../interfaces";
1514
import { clearForm } from "../utils/sharedFunctions";
1615
import { FetchErrorData } from "@thepalaceproject/web-opds-client/lib/interfaces";
@@ -27,6 +26,7 @@ export interface ServiceEditFormProps<T> {
2726
extraFormSection?: any;
2827
extraFormKey?: string;
2928
adminLevel?: number;
29+
libraryRemovalAllowed?: (library: LibraryWithSettingsData) => boolean;
3030
}
3131

3232
export interface ServiceEditFormState {
@@ -345,6 +345,7 @@ export default class ServiceEditForm<
345345
<WithRemoveButton
346346
disabled={disabled}
347347
onRemove={() => this.removeLibrary(library)}
348+
confirmRemoval={() => this.isLibraryRemovalPermitted(library)}
348349
ref={library.short_name}
349350
>
350351
{this.props.data &&
@@ -588,6 +589,31 @@ export default class ServiceEditForm<
588589
return this.state.expandedLibraries.indexOf(library.short_name) !== -1;
589590
}
590591

592+
/**
593+
* Controls whether library associations may be removed via UI interaction.
594+
* Subclasses may override this method to control removal.
595+
* @param library The library to remove.
596+
*/
597+
isLibraryRemovalPermitted(library: LibraryWithSettingsData): boolean {
598+
// library should be provided on every call.
599+
// It is not used here, but might be in subclass implementations.
600+
// Removal is permitted by default.
601+
return true;
602+
}
603+
604+
/**
605+
* Removes the association with the given library from a service's state.
606+
*
607+
* NB: This method is used indirectly via `onRemove` by `sharedFunctions.clearForm`
608+
* to clean up form state; therefore, care should be taken to avoid any side
609+
* effects not needed for updating the state. Anything done by this method
610+
* can happen whenever user interaction or `clearForm` triggers `onRemove`.
611+
*
612+
* Update or override `isLibraryRemovalPermitted` to control library removal
613+
* via UI interaction.
614+
*
615+
* @param library
616+
*/
591617
removeLibrary(library) {
592618
const libraries = this.state.libraries.filter(
593619
(stateLibrary) => stateLibrary.short_name !== library.short_name

src/components/WithRemoveButton.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import TrashIcon from "./icons/TrashIcon";
55
export interface WithRemoveButtonProps {
66
disabled: boolean;
77
onRemove: () => void;
8+
confirmRemoval?: () => boolean;
89
}
910

1011
/** When wrapped around an element, renders a remove button next to the element. */
@@ -37,6 +38,12 @@ export default class WithRemoveButton extends React.Component<
3738

3839
onClick(e: Event) {
3940
e.preventDefault();
41+
42+
// Don't remove if confirmation function is present and returns false.
43+
if (this.props.confirmRemoval && !this.props.confirmRemoval()) {
44+
return;
45+
}
46+
// Otherwise, we can proceed with removal.
4047
!this.props.disabled && this.props.onRemove();
4148
}
4249
}

src/components/__tests__/ServiceEditForm-test.tsx

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,4 +1009,79 @@ describe("ServiceEditForm", () => {
10091009
expect(nameInput.props().value).to.equal("new service");
10101010
});
10111011
});
1012+
1013+
describe("library removal confirmation", () => {
1014+
let serviceEditFormInstance;
1015+
let isLibraryRemovalPermittedSpy;
1016+
let removeLibrarySpy;
1017+
1018+
beforeEach(() => {
1019+
save = stub().returns(new Promise<void>((resolve) => resolve()));
1020+
wrapper = mount(
1021+
<TestServiceEditForm
1022+
disabled={false}
1023+
data={servicesData}
1024+
save={save}
1025+
item={serviceData}
1026+
urlBase={urlBase}
1027+
listDataKey="services"
1028+
/>
1029+
);
1030+
serviceEditFormInstance = wrapper.instance();
1031+
removeLibrarySpy = spy(serviceEditFormInstance, "removeLibrary");
1032+
});
1033+
1034+
afterEach(() => {
1035+
isLibraryRemovalPermittedSpy.restore();
1036+
removeLibrarySpy.restore();
1037+
});
1038+
1039+
const shouldRemoveLibrary = () => {
1040+
const libraryToRemove = serviceData.libraries[0];
1041+
const removeButton = wrapper.find(WithRemoveButton).at(0).find("button.remove-btn");
1042+
1043+
removeButton.simulate("click");
1044+
1045+
expect(isLibraryRemovalPermittedSpy.calledOnce).to.be.true;
1046+
expect(isLibraryRemovalPermittedSpy.calledWith(libraryToRemove)).to.be.true;
1047+
expect(isLibraryRemovalPermittedSpy.returned(true)).to.be.true;
1048+
1049+
// Library removal should be performed.
1050+
expect(removeLibrarySpy.calledOnce).to.be.true;
1051+
expect(removeLibrarySpy.calledWith(libraryToRemove)).to.be.true;
1052+
1053+
// Ensure that the library was removed from state.
1054+
wrapper.update();
1055+
expect(wrapper.state("libraries")).to.deep.equal([]);
1056+
};
1057+
1058+
it("should allow removal by default", () => {
1059+
isLibraryRemovalPermittedSpy = spy(serviceEditFormInstance, "isLibraryRemovalPermitted");
1060+
shouldRemoveLibrary();
1061+
});
1062+
1063+
it("should allow remove if isLibraryRemovalPermitted returns true", () => {
1064+
isLibraryRemovalPermittedSpy = stub(serviceEditFormInstance, "isLibraryRemovalPermitted").returns(true);
1065+
shouldRemoveLibrary();
1066+
});
1067+
1068+
it("should not allow remove if isLibraryRemovalPermitted returns false", () => {
1069+
isLibraryRemovalPermittedSpy = stub(serviceEditFormInstance, "isLibraryRemovalPermitted").returns(false);
1070+
const libraryToRemove = serviceData.libraries[0];
1071+
const removeButton = wrapper.find(WithRemoveButton).at(0).find("button.remove-btn");
1072+
1073+
removeButton.simulate("click");
1074+
1075+
expect(isLibraryRemovalPermittedSpy.calledOnce).to.be.true;
1076+
expect(isLibraryRemovalPermittedSpy.calledWith(libraryToRemove)).to.be.true;
1077+
expect(isLibraryRemovalPermittedSpy.returned(false)).to.be.true;
1078+
1079+
// Library disassociation should NOT be performed.
1080+
expect(removeLibrarySpy.notCalled).to.be.true;
1081+
1082+
// Ensure that the library was NOT removed from state.
1083+
wrapper.update();
1084+
expect(wrapper.state("libraries")).to.deep.equal(serviceData.libraries);
1085+
});
1086+
});
10121087
});

src/components/__tests__/WithRemoveButton-test.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,27 @@ describe("WithRemoveButton", () => {
4343
removeBtn.simulate("click");
4444
expect(onRemove.callCount).to.equal(0);
4545
});
46+
47+
it("calls onRemove when confirmation returns true", () => {
48+
const confirmRemoval = stub().returns(true);
49+
wrapper.setProps({ confirmRemoval });
50+
const removeBtn = wrapper.find(".remove-btn").hostNodes();
51+
52+
removeBtn.simulate("click");
53+
54+
expect(confirmRemoval.callCount).to.equal(1);
55+
expect(onRemove.callCount).to.equal(1);
56+
});
57+
58+
it("does not call onRemove when confirmation returns false", () => {
59+
const confirmRemoval = stub().returns(false);
60+
wrapper.setProps({ confirmRemoval });
61+
const removeBtn = wrapper.find(".remove-btn").hostNodes();
62+
63+
removeBtn.simulate("click");
64+
65+
expect(confirmRemoval.callCount).to.equal(1);
66+
expect(onRemove.callCount).to.equal(0);
67+
});
4668
});
4769
});

0 commit comments

Comments
 (0)