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

2234 & 2235/configure storage destination #2358

Closed
wants to merge 14 commits into from

Conversation

LKCSmith
Copy link
Contributor

@LKCSmith LKCSmith commented Jan 25, 2023

Closes #2234
Closes #2235

NOTE:

  • Tooltips will be handled in a subsequent ticket
  • The local storage path currently doesn't do anything except save it as the auth type
  • This should be merged after the BE work is completed as it uses new storage API

Screen Shot 2023-01-24 at 1 37 43 PM

Note: The onClick behavior for the messaging box won't do anything yet until the subsequent issue is complete. This feature is flagged.

TODO:

  • Connect and Test against BE once that work is complete
  • Fix subnav highlight

Code Changes

  • Add feature flag for storage UI work
  • Add path for messaging and storage configuration work
  • Form for local and s3 storage
  • s3 path to save storage details and auth secrets for configuration

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

Description Of Changes

Write some things here about the changes and any potential caveats

@LKCSmith LKCSmith self-assigned this Jan 25, 2023
@LKCSmith LKCSmith changed the title 2235/configure storage destination 2234 & 2235/configure storage destination Jan 27, 2023
);
if (
href === "/privacy-requests/configure" &&
!features.flags.privacyRequestsConfiguration
Copy link
Contributor

Choose a reason for hiding this comment

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

This toggling of certain routes based on feature flags should be done from the nav config helpers themselves instead of within component markup. Check out configureNavGroups and its unit tests -- it already takes in some features to decide what should be shown.

@@ -353,6 +353,32 @@ export const privacyRequestApi = createApi({
}),
invalidatesTags: ["Notification"],
}),
getStorageDetails: build.query<any, any>({
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no does this whole file use any for its query params/returns? I'd expect the models to be generated by openapi.

Copy link
Contributor

@ssangervasi ssangervasi left a comment

Choose a reason for hiding this comment

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

We'll want some cypress tests, too.

@LKCSmith LKCSmith removed the request for review from chriscalhoun1974 January 29, 2023 00:30
@codecov
Copy link

codecov bot commented Jan 29, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@159b235). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2358   +/-   ##
=======================================
  Coverage        ?   88.55%           
=======================================
  Files           ?      327           
  Lines           ?    15714           
  Branches        ?     4348           
=======================================
  Hits            ?    13915           
  Misses          ?     1644           
  Partials        ?      155           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@LKCSmith LKCSmith closed this Feb 1, 2023
@NevilleS NevilleS deleted the 2235/configure-storage-destination branch May 16, 2024 13:00
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.

Configure storage destination via UI "Configure your privacy requests" screen
2 participants