New subject: [Bug 36369] Make APIClient be more useful

20 Mar 2024

      https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369

            Bug ID: 36369
           Summary: Make APIClient be more useful
 Change sponsored?: ---
           Product: Koha
           Version: master
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5 - low
         Component: Architecture, internals, and plumbing
          Assignee: koha-bugs@lists.koha-community.org
          Reporter: tomascohen@gmail.com
        QA Contact: testopia@bugs.koha-community.org

I file this report for discussion.

In my opinion, there's too much duplicated code on the API client libraries.
And the APIClient "class" is just a shortcut to avoid instantiating the
module-specific API clients:

```javascript
import ERMAPIClient from "./erm-api-client";
...

export const APIClient = {
    erm: new ERMAPIClient(),
```

Then used everywhere like this:

```javascript
import { APIClient } from "../../fetch/api-client.js"
...
            const client = APIClient.erm
            await client.agreements.count().then(
```

IMHO, we need a real APIClient class that would implement generic methods like:

```javascript
    create: params =>
        this.post({
            endpoint: params.endpoint ? params.endpoint : "/",
            body: params.resource,
            headers: params.headers
        }),
    ...
    getAll: params =>
        this.get({
            endpoint: params.endpoint ? params.endpoint : "/",
            params.query,
            headers: params.headers,
        }),
...
    count: params =>
        this.count({
            endpoint: params.endpoint ? params.endpoint : "/",
                "?" +
                new URLSearchParams({
                    _page: 1,
                    _per_page: 1,
                    ...(params.query && { q: JSON.stringify(params.query) }),
                }),
        }),
```

and the API Client classes (like ERMAPIClient) should inherit from it, somehow.
For doing so, we will face another design roadblock:

While I uderstand that ERMAPIClient is a single place to have all the ERM
module-needed API actions, it still feels weird from an OOP perspective how
things are organized.

I feel like we should have individual classes, like:

```javascript
export RecordSourcesAPIClient extends APIClient
```

and then have a new type of class along this lines:

```javascript
import CitiesAPIClient from "./cities-api-client";
import RecordSourcesAPIClient from "./record-sources-api-client";
...

export const AdminAPIClient = {
    cities: new CitiesAPIClient(),
    record_sources: new RecordSourcesAPIClient(),
    ...
};
```

this way, we will avoid repeating ourselves (less bugs!) and the code is (I
expect) more reusable.

I also think the `embed` handling should just be a list for the different
methods, like:

embed: [ 'patron', 'patron.category' ]

and generically handled by APIClient. No need to build the headers manually
everywhere.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.

[Bug 36369] New: Make APIClient be more useful

tags

participants (1)