Skip to content

Commit

Permalink
Prevent redundant /admin/libraries requests when opening config scree…
Browse files Browse the repository at this point in the history
…n. (#17)
  • Loading branch information
Ray Lee authored Dec 17, 2021
1 parent 3def790 commit 0817390
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 9 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
## Changelog

### December 14, 2021
### December 16, 2021

#### Updated

- Updated branding and styling for The Palace Project.
- Updated node-sass and sass-loader dependency versions to reduce the number of high risk vulnerabilities.
- Removed Analytics tab from System Configuration for librarians and library managers, and added Admins tab.
- Relabeled "library manager" role to "administrator", and "librarian" role to "user".
- Removed redundant requests to retrieve libraries when opening the system configuration screen.

### v0.5.5

Expand Down
6 changes: 4 additions & 2 deletions src/components/EditableConfigList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ export abstract class GenericEditableConfigList<
}

UNSAFE_componentWillMount() {
if (this.props.fetchData) {
this.props.fetchData();
const { fetchData, isFetching } = this.props;

if (fetchData && !isFetching) {
fetchData();
}
}

Expand Down
13 changes: 7 additions & 6 deletions src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as palaceLogoUrl from "../images/PalaceCollectionManagerLogo.svg";
import title from "../utils/title";

export interface HeaderStateProps {
isFetchingLibraries?: boolean;
libraries?: LibraryData[];
}

Expand Down Expand Up @@ -203,8 +204,10 @@ export class Header extends React.Component<HeaderProps, HeaderState> {
}

UNSAFE_componentWillMount() {
if (this.props.fetchLibraries) {
this.props.fetchLibraries();
const { fetchLibraries, isFetchingLibraries } = this.props;

if (fetchLibraries && !isFetchingLibraries) {
fetchLibraries();
}
}

Expand Down Expand Up @@ -297,10 +300,8 @@ export class Header extends React.Component<HeaderProps, HeaderState> {

function mapStateToProps(state, ownProps) {
return {
libraries:
state.editor.libraries &&
state.editor.libraries.data &&
state.editor.libraries.data.libraries,
isFetchingLibraries: state.editor.libraries?.isFetching,
libraries: state.editor.libraries?.data?.libraries,
};
}

Expand Down
20 changes: 20 additions & 0 deletions src/components/__tests__/EditableConfigList-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,26 @@ describe("EditableConfigList", () => {
expect(editItem.callCount).to.equal(1);
});

it("does not fetch data on mount if a fetch is already in progress", () => {
// Expect one pre-existing call to fetchData from the component mounted in beforeEach()...
expect(fetchData.callCount).to.equal(1);

wrapper = shallow(
<ThingEditableConfigList
data={thingsData}
fetchData={fetchData}
editItem={editItem}
deleteItem={deleteItem}
csrfToken="token"
isFetching
/>,
{ context: { admin: systemAdmin } }
);

// ...but no more!
expect(fetchData.callCount).to.equal(1);
});

it("fetches data again on save", async () => {
wrapper.setProps({ editOrCreate: "create" });
const form = wrapper.find(ThingEditForm);
Expand Down
13 changes: 13 additions & 0 deletions src/components/__tests__/Header-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,19 @@ describe("Header", () => {
expect(fetchLibraries.callCount).to.equal(1);
});

it("does not fetch libraries on mount if a fetch is already in progress", () => {
const fetchLibraries = stub();

wrapper = shallow(
<Header fetchLibraries={fetchLibraries} isFetchingLibraries />,
{
context: { admin: libraryManager },
}
);

expect(fetchLibraries.callCount).to.equal(0);
});

it("changes library", async () => {
const libraries = [
{ short_name: "nypl", name: "NYPL" },
Expand Down

0 comments on commit 0817390

Please sign in to comment.