Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Proxies: rest queries and notifications #1240

Merged
merged 2 commits into from
Aug 3, 2023
Merged

✨ Proxies: rest queries and notifications #1240

merged 2 commits into from
Aug 3, 2023

Conversation

gildub
Copy link
Contributor

@gildub gildub commented Aug 1, 2023

In continuation of work already done to align, feature by feature, here the proxy configuration to :

  • Have it's REST function to use axios instead of APIConfig and uUse React Query consitenlty
  • Use Notificationinstead forAlert`.

@gildub gildub requested a review from ibolton336 August 1, 2023 15:47
@gildub gildub self-assigned this Aug 1, 2023
@gildub gildub requested a review from sjd78 August 1, 2023 15:47
@gildub gildub changed the title ✨ Proxies: rest, queries and notifications ✨ Proxies: rest queries and notifications Aug 1, 2023
@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 80.00% and project coverage change: +0.01% 🎉

Comparison is base (9d59bd2) 44.03% compared to head (d0bf601) 44.04%.
Report is 1 commits behind head on main.

❗ Current head d0bf601 differs from pull request most recent head 087310c. Consider uploading reports for the commit 087310c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1240      +/-   ##
==========================================
+ Coverage   44.03%   44.04%   +0.01%     
==========================================
  Files         177      177              
  Lines        4524     4516       -8     
  Branches     1009     1038      +29     
==========================================
- Hits         1992     1989       -3     
+ Misses       2521     2452      -69     
- Partials       11       75      +64     
Flag Coverage Δ
client 44.04% <80.00%> (+0.01%) ⬆️
server ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
client/src/app/queries/proxies.ts 73.33% <71.42%> (+6.66%) ⬆️
client/src/app/api/rest.ts 54.67% <75.00%> (+0.05%) ⬆️
client/src/app/pages/proxies/proxy-form.tsx 36.29% <85.71%> (+1.83%) ⬆️

... and 27 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

export const useUpdateProxyMutation = (onSuccess: any) => {
const [putResult, setPutResult] = useState<any>(null);
export const useUpdateProxyMutation = (
onSuccess: (res: any) => void,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a type here for the response if it is needed.

@@ -309,22 +309,6 @@ export const deleteIdentity = (identity: Identity): AxiosPromise => {
return APIClient.delete(`${IDENTITIES}/${identity.id}`);
};

export const getProxies = (): AxiosPromise<Array<Proxy>> => {
return APIClient.get(`${PROXIES}`, jsonHeaders);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Member

@ibolton336 ibolton336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment. Looking good though!

@gildub gildub requested a review from ibolton336 August 3, 2023 13:15
Signed-off-by: Gilles Dubreuil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants