refactor(test): proper error handling (#128)

src/model.ts

@@ -8,20 +8,39 @@ interface Subtheme {
   name: string
 }
 
-interface DiscussionRequest {
-  title: string,
-  comment: string,
-  subject: DiscussionSubject,
+interface Request {
+  method: string
+  url: string
+  data?: Record<string, any>
+}
+
+interface Response {
+  status: number
+  data?: Record<string, any>
+  error?: { message: string }
+}
+
+interface DiscussionParams {
+  title: string
+  comment: string
+  subject: DiscussionSubject
 }
 
 interface DiscussionSubject {
-  class: "Topic"
-  "id": string
+  class: 'Topic'
+  id: string
 }
 
-interface DiscussionResponse {
-  status: 200
-  data: DiscussionRequest
+interface DiscussionResponse extends Response {
+  status: 200 | number
+  data?: Partial<DiscussionParams>
 }
 
-export type { Theme, Subtheme, DiscussionResponse }
+export type {
+  Theme,
+  Subtheme,
+  Request,
+  Response,
+  DiscussionParams,
+  DiscussionResponse
+}

src/services/api/DatagouvfrAPI.js → src/services/api/DatagouvfrAPI.ts

@@ -3,7 +3,9 @@ import { toast } from 'vue3-toastify'
 import { useLoading } from 'vue-loading-overlay'
 
 import config from '@/config'
+import type { Response } from '@/model'
 
+// FIXME: the client should not depend be aware of the store.
 import { useUserStore } from '../../store/UserStore'
 
 const $loading = useLoading()
@@ -21,7 +23,7 @@ instance.interceptors.request.use(
 
     return config
   },
-  (error) => Promise.reject(error)
+  async (error) => await Promise.reject(error)
 )
 
 /**
@@ -73,14 +75,16 @@ export default class DatagouvfrAPI {
    */
   async makeRequestAndHandleResponse(url, method = 'get', params = {}) {
     const loader = $loading.show()
-    return this.request(url, method, params)
+    return await this.request(url, method, params)
       .catch((error) => {
-        if (error && error.message) {
+        if (error?.message) {
           toast(error.message, { type: 'error', autoClose: false }) // TODO: Refacto to handle the error
           return error.response
         }
       })
-      .finally(() => loader.hide())
+      .finally(() => {
+        loader.hide()
+      })
   }
 
   /**
@@ -141,6 +145,23 @@ export default class DatagouvfrAPI {
     )
   }
 
+  /**
+   * Base function for HTTP calls (without error handling)
+   *
+   * @todo Remove this function once all API calls are all handled this way:
+   *       leaving the error handling to the caller (store).
+   *
+   * @param {string} url
+   * @param {object} data
+   * @returns {Promise<Response>}
+   */
+  async _create(url: string, data = {}): Promise<Response> {
+    return await this.httpClient.post(url, data).then(
+      (response) => response,
+      (error) => this.#handleError(error)
+    )
+  }
+
   /**
    * Update an entity (PUT)
    *
@@ -160,17 +181,19 @@ export default class DatagouvfrAPI {
    * Delete an entity (DELETE)
    *
    * @param {string} entityId - A UUID entity id
-   * @returns {Promise}
+   * @returns {Promise<Response>}
    */
-  async delete(entityId) {
-    return this.httpClient.delete(`${this.url()}/${entityId}/`).then(
+  async delete(entityId: string): Promise<Response> {
+    return await this.httpClient.delete(`${this.url()}/${entityId}/`).then(
       (response) => response,
       (error) => this.#handleError(error)
     )
   }
 
-  #handleError({ response, message }) {
-    if (response) return { status: response.status }
-    return { message }
+  #handleError({ response, message }): Response {
+    return {
+      ...(response && { status: response.status }),
+      ...(message && { error: { message } })
+    }
   }
 }

src/services/api/__tests__/DatagouvfrAPI.test.js

@@ -80,8 +80,8 @@ test('delete when 404', async ({ client }) => {
 })
 
 test('delete something else', async ({ client }) => {
-  const { message } = await client.delete(networkError)
-  expect(message).toMatch(/network error/i)
+  const { error } = await client.delete(networkError)
+  expect(error.message).toMatch(/network error/i)
 })
 
 test('raw list', async ({ client }) => {

src/services/api/__tests__/DiscussionsAPI.test.js

@@ -11,38 +11,39 @@ import {
   test
 } from 'vitest'
 
-import DiscussionsAPI from '@/services/api/DiscussionsAPI'
+import DiscussionsAPI from '@/services/api/resources/DiscussionsAPI'
 
 const baseUrl = 'https://example.lol'
 const version = '1234'
 const endpoint = 'discussions'
-const datasetId = 'your-dataset-id'
-const page = 1
 
-const server = setupServer(
-  http.get(`${baseUrl}/${version}/${endpoint}/?for=${datasetId}&page=${page}`, () => {
-    return HttpResponse.json([], { status: 200 })
-  }),
+const discussionRequest = {
+  title: 'Title of the discussion',
+  comment: 'This is a discussion.',
+  subject: {
+    class: 'Topic',
+    id: 'id123'
+  }
+}
 
-  http.post(`${baseUrl}/${version}/${endpoint}/`, (req, res, ctx) => {
-    const data = req.body
-    // Assuming you have some logic to validate the request and return a response
-    return res(ctx.json(data), ctx.status(201))
+const server = setupServer(
+  http.post(`${baseUrl}/${version}/${endpoint}/`, () => {
+    return HttpResponse.json(discussionRequest, { status: 200 })
   })
 )
 
 beforeAll(() => {
   server.listen()
 })
 
-beforeEach(() => {
+beforeEach(async (context) => {
   const httpClient = axios.create()
   httpClient.defaults.proxy = false
   setActivePinia(createPinia())
   context.client = new DiscussionsAPI({
     baseUrl,
     version,
-    httpClient,
+    httpClient
   })
 })
 
@@ -54,22 +55,7 @@ afterAll(() => {
   server.close()
 })
 
-test('get a discussion', async ({ client }) => {
-  // TODO: Test on get a discussion
-})
-
 test('create a discussion', async ({ client }) => {
-  const discussionData = {
-    title: 'Title of the discussion',
-    comment: 'This is a discussion.',
-    subject: {
-      class: 'Topic',
-      id: 'id123'
-    }
-  }
-
-  expect(response.id).toBeDefined()
-  expect(response.title).toEqual(discussionData.title)
-  expect(response.comment).toEqual(discussionData.comment)
-  expect(response.subject).toEqual(discussionData.subject)
+  const { data } = await client.create(discussionRequest)
+  expect(data.title).toEqual(discussionRequest.title)
 })

src/services/api/resources/DiscussionsAPI.ts

@@ -1,5 +1,6 @@
+import type { DiscussionParams, DiscussionResponse } from '@/model'
+
 import DatagouvfrAPI from '../DatagouvfrAPI'
-import type { DiscussionRequest } from '@/model'
 
 export default class DiscussionsAPI extends DatagouvfrAPI {
   endpoint = 'discussions'
@@ -17,24 +18,12 @@ export default class DiscussionsAPI extends DatagouvfrAPI {
   }
 
   /**
-   * Base function for HTTP calls (without error handling)
+   * Create a discussion (POST)
    *
-   * @param {string} url
-   * @param {string} method
-   * @param {object} params
-   * @returns {Promise}
-   */
-  async _request(url, method = 'get', params = {}) {
-    return await this.httpClient[method](url, params)
-  }
-
-  /**
-   * Create an discussion (POST)
-   *
-   * @param {object} data
-   * @returns {object}
+   * @param {DiscussionParams} data
+   * @returns {Promise<DiscussionResponse>}
    */
-  async create(data: DiscussionRequest): Promise<DiscussionRequest> {
-    return await this.request(`${this.url()}/`, 'post', data)
+  async create(data: DiscussionParams): Promise<DiscussionResponse> {
+    return await this._create(`${this.url()}/`, data)
   }
 }