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.