ENG-1868 Adds admin UI support for v3 API #6933
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![greptile-apps[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds Admin UI support for the Fides v3 API by creating a new RTK Query API slice for v3 endpoints and adding proxy configuration in Next.js.
Changes:
- Created new
v3Apislice with consent preference endpoints (savePrivacyPreferences,getCurrentPreferences) - Added v3 API proxy rewrites in
next.config.jsto route/api/v3/*requests to the backend server - Integrated v3Api reducer and middleware into the Redux store
Issue:
- The
v3Api.reducerPathis missing from the persist blacklist, which could cause phantom subscriptions from cached API data
Confidence Score: 4/5
- This PR is safe to merge after fixing the persist blacklist issue
- The implementation follows the existing pattern for API slices (similar to healthApi), but missing v3Api from the persist blacklist could cause issues with phantom subscriptions. Once fixed, the changes are straightforward and low-risk.
- clients/admin-ui/src/app/store.ts requires adding v3Api to the persist blacklist
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| clients/admin-ui/src/features/poc/privacy-notices-sandbox/v3-api.ts | 4/5 | Created v3 API slice with consent endpoints - missing v3Api from persist blacklist |
| clients/admin-ui/src/app/store.ts | 4/5 | Added v3Api reducer and middleware - missing v3Api.reducerPath from persist blacklist |
Additional Comments (1)
-
clients/admin-ui/src/app/store.ts, line 139-143 (link)logic:
v3Api.reducerPathshould be added to the blacklist to prevent the api cache from being persisted and restored, which could cause phantom subscriptions
4 files reviewed, 1 comment
tvandort
left a comment
tvandort
left a comment
There was a problem hiding this comment.
Do we need to change our env vars in our env file after this is merged?
| export interface ConsentPreferenceCreate { | ||
| notice_key: string; | ||
| notice_history_id: string; | ||
| value: "opt_in" | "opt_out" | "acknowledge"; | ||
| experience_config_history_id?: string | null; | ||
| meta?: Record<string, any> | null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Are these automatically generated? Will they be in the future if they are not right now?
There was a problem hiding this comment.
these weren't but I think they should be. Maybe I need to run that type generation thing somewhere
| export const v3Api = createApi({ | ||
| reducerPath: "v3Api", | ||
| baseQuery: fetchBaseQuery({ | ||
| baseUrl: "/api/v3", | ||
| prepareHeaders: (headers, { getState }) => { | ||
| const { token } = (getState() as RootState).auth; | ||
| addCommonHeaders(headers, token); | ||
| return headers; | ||
| }, | ||
| }), | ||
| endpoints: () => ({}), | ||
| }); |
There was a problem hiding this comment.
Might be worthwhile to colocate this with api.slice.ts as I suspect it might live longer than we expect.
There was a problem hiding this comment.
done!
also answering the question
Do we need to change our env vars in our env file after this is merged?
I don't see why you would, I didn't need to change any env vars
| // V3 API proxying for consent sandbox | ||
| { | ||
| source: `/api/v3/:path`, | ||
| destination: `${process.env.NEXT_PUBLIC_FIDESCTL_API_SERVER}/api/v3/:path`, | ||
| }, | ||
| { | ||
| source: `/api/v3/:first/:second*`, | ||
| destination: `${process.env.NEXT_PUBLIC_FIDESCTL_API_SERVER}/api/v3/:first/:second*`, | ||
| }, |
There was a problem hiding this comment.
Do you have a notion for why this is needed? I know we're mirroring what was done for v1 api (which is good) but I'm curious why both of these proxies are necessary.
There was a problem hiding this comment.
I have no idea 😆 I hadn't noticed there were two until you pointed it out 😅
There was a problem hiding this comment.
Ticket ENG-1868
Description Of Changes
Adds Admin UI support for the Fides v3 API by updating the next config (to include v3 routing) and the
store.tsfile.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works