Skip to content
This repository has been archived by the owner on Jun 18, 2020. It is now read-only.

Request meta data #371

Merged
merged 16 commits into from
Mar 31, 2020
Merged

Request meta data #371

merged 16 commits into from
Mar 31, 2020

Conversation

frontendphil
Copy link
Contributor

@frontendphil frontendphil commented Mar 26, 2020

BREAKING

API function fetch, create, update, and remove must be declared differently now. The callApi method will be injected now. In addition to the new addMetaData action, this allows us to set custom headers for all kraken calls.

Additionally, the new apiBase property will be injected which can also be set through the addMetaData action. With that, you can provide an API base to all calls.

// old
import { callApi } from '@signavio/kraken'

const API_BASE = '/api/v1'

export const fetch = ({ id }) => callApi(`${API_BASE}/users/${id}`)

// new
export const fetch = (callApi, apiBase) => ({ id }) => callApi(`${apiBase}/users/${id}`)

@SilentGert
Copy link
Contributor

How would you handle admin calls now which use an extended apiBase? Does every type specify it separately?

@frontendphil
Copy link
Contributor Author

frontendphil commented Mar 26, 2020

How would you handle admin calls now which use an extended apiBase? Does every type specify it separately?

@SilentGert yes this would make it more explicit right now. Like:

import { API_BASE } from '../somewhere'

const fetch = (callApi) => ({ id }) => callApi(`${API_BASE}/admin/users/${id}`)

In our codebase, we could then get rid of all those wrappers that try to find a good name for essentially added a different prefix to the URL :)

@frontendphil frontendphil marked this pull request as ready for review March 26, 2020 16:03
@frontendphil frontendphil changed the title Improvement/meta data Request meta data Mar 26, 2020
@gisaklement
Copy link
Contributor

gisaklement commented Mar 27, 2020

This is a good candidate for pair/mob reviewing!

@codecov-io
Copy link

codecov-io commented Mar 27, 2020

Codecov Report

Merging #371 into master will decrease coverage by 1.09%.
The diff coverage is 93.39%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #371     +/-   ##
=========================================
- Coverage   92.33%   91.24%   -1.1%     
=========================================
  Files          55       57      +2     
  Lines        1919     2091    +172     
  Branches      375      408     +33     
=========================================
+ Hits         1772     1908    +136     
- Misses        138      174     +36     
  Partials        9        9
Impacted Files Coverage Δ
src/flowTypes/api.js 100% <ø> (ø) ⬆️
src/index.js 75.86% <0%> (-24.14%) ⬇️
src/flowTypes/index.js 33.33% <0%> (+0.72%) ⬆️
src/hooks/useApiBase.js 0% <0%> (ø)
src/sagas/watchRemoveDispatch.js 100% <100%> (ø) ⬆️
src/sagas/watchCreateDispatch.js 100% <100%> (ø) ⬆️
src/sagas/watchFetchDispatch.js 100% <100%> (ø) ⬆️
src/reducers/metaDataReducer.js 100% <100%> (ø)
src/sagas/index.js 100% <100%> (ø) ⬆️
src/reducers/index.js 100% <100%> (ø) ⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09c4fd4...0ba0cd6. Read the comment docs.

@frontendphil frontendphil merged commit 4bf2448 into master Mar 31, 2020
@frontendphil frontendphil deleted the improvement/meta-data branch March 31, 2020 10:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants