Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Datastore Connection Filtering #691

Merged
merged 48 commits into from
Jun 23, 2022
Merged

Datastore Connection Filtering #691

merged 48 commits into from
Jun 23, 2022

Conversation

TheAndrewJackson
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson commented Jun 21, 2022 •
edited
Loading

❗ Dependent on #674, merge that first.

Purpose

Expand on the work that was done in #674 by adding filters to control what connections are displayed on the page.

Changes

  • Add 2 new custom dropdown components
  • Update datastore slice to accommodate all of the filter keys
  • refactor mapFiltersToSearchParams

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #669

TheAndrewJackson and others added 30 commits June 10, 2022 16:30
@TheAndrewJackson
Refactor routes into enums and create connections page
7c17f15
@TheAndrewJackson
Test switching back to double quotes
5797f4e
@TheAndrewJackson
Convert back to double quotes
cf5acd6
@TheAndrewJackson
Add placeholder connection filters
7bc6068
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
2720ea4
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
8f0a9be
@TheAndrewJackson
Set up api scaffolding
45a9c7f
@TheAndrewJackson
Get basic grid going
852e326
@TheAndrewJackson
Initial grid card styling
557b3a6
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
04c91b6
@TheAndrewJackson
Fix simple eslint issues
2e13ec3
@TheAndrewJackson
Add development config back in
7b0c848
@TheAndrewJackson
Finish draft of card
4b112de
@TheAndrewJackson
Add working test button and landing page
5928822
@TheAndrewJackson
Add pagination and small fixes
2f20126
@TheAndrewJackson
Fix testing issues
a7dd382
@TheAndrewJackson
Add auth tests for datastore connection page
939c021
@TheAndrewJackson
run formatter
d9376fd
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
4e78fe4
@TheAndrewJackson
Update changelog
477e40e
update the create_test_data command to add connectionconfigs
acaad91
@TheAndrewJackson
Disable create buttons & fix text overflow
09778d1
@TheAndrewJackson
Update filter dropdown values
5fad702
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
b9773be
@TheAndrewJackson
Fix test timestamp bug
d9bd8fb
@TheAndrewJackson
Remove development variable
451fb52
@TheAndrewJackson
Add working filter dropdowns
f7da9f2
@TheAndrewJackson
Add outside click hook & polish things
d7afb8c
@TheAndrewJackson
Merge branch 'main' into 601_datastore_management_landing_page
b23fa48
@TheAndrewJackson
Fix imports
de09d9c
@TheAndrewJackson TheAndrewJackson marked this pull request as ready for review June 21, 2022 20:59
@TheAndrewJackson TheAndrewJackson changed the title 669 datastore connection filtering Datastore Connection Filtering Jun 21, 2022
@TheAndrewJackson TheAndrewJackson mentioned this pull request Jun 22, 2022
Merged
10 tasks
Base automatically changed from 601_datastore_management_landing_page to main June 22, 2022 14:56
@eastandwestwind
Copy link
Contributor

eastandwestwind commented Jun 22, 2022 •
edited
Loading

Functionally looking good, @TheAndrewJackson , just a couple UI questions/nitpicks:

  1. Just confirming we don't want "clearing" functionality for Database, Testing Status, and Status drop-downs, like we have for Datastore Type?

  2. Also confirming we want to forgo the display of number of items selected / item selected depending on if it's a multi-select drop-down?

  3. When there are datastore cards next to each other, there is a double outline instead of a single outline. This isn't blocking, but would be fine in a follow-up, even in next release imo. But I'll defer to @adriaaaa

Screen Shot 2022-06-22 at 12 40 41 PM

  1. When only 1 or 2 connections exist, the datastore cards stretch to fill the screen. In the mocks they're a consistent 1/3 of the screen.

Screen Shot 2022-06-22 at 12 42 45 PM

  1. For the dropdown UI itself, only the small arrow element is clickable. I want to confirm that this is intended?

Screen Shot 2022-06-22 at 12 51 46 PM

  1. Should we grey out the edit/disable/delete options for now since the don't do anything?

Screen Shot 2022-06-22 at 12 53 32 PM

@TheAndrewJackson
Copy link
Contributor Author

Good eye! These are all great finds. Since the functionality of the filtering is working ok would you be comfortable addressing some of these in follow up PRs? @eastandwestwind

  1. Good question. I guess I didn't add it in because I didn't see it in the mock ups. I can see it being useful. What do you think? @adriaaaa
  2. I think it does that already. Were you running into issues? This is what I'm seeing. Screen Shot 2022-06-22 at 15 39 15
  3. I agree. Good follow up item.
  4. Interesting. I can see that. The way that I implemented the responsive grid does allow the cards to grow fairly big. It works well when there are more cards but does look a little weird when there is one. I assumed that they would grow because the mockups all had the same screen width.
  5. I did implement it that way but it can be changed.
  6. Both Disable and Delete have been merged in so they should work. Edit could be removed. Thoughts @adriaaaa?

@eastandwestwind
Copy link
Contributor

eastandwestwind commented Jun 22, 2022 •
edited
Loading

  1. You're right I missed this!
  2. This is fine for follow-up if it's something we wish to fix from product perpective
  3. Hmm looks like I got delete to work after page refresh. Not sure what the issue was there. But disable is still not working. Can you confirm it's still working on your end?

Waiting on @adriaaaa to confirm whether we should disable/grey the "edit" button for item 6.

@TheAndrewJackson
Copy link
Contributor Author

I'm able to disable and enable connections on the most recent commit. Maybe pulling down latest will help?

Screen.Recording.2022-06-22.at.16.06.45.mov

@adriaaaa
Copy link

adriaaaa commented Jun 22, 2022 •
edited
Loading

@eastandwestwind @TheAndrewJackson
question 1: For clearing functionality, can a user still "clear" those filters by just selecting like "All Statuses"? If so, this is fine.

question 6: Yeah since we don't support editing functionality right now we should either remove "edit" as an option or grey it out/disabling it. I'd be ok with disabling it if that is easy enough.

@eastandwestwind
Copy link
Contributor

question 1- I don't see a way to clear those filters after selecting, for example, SaaS for system type. there's no way to deselect to view all again. Is this a blocker for release @adriaaaa ?

question 6- I was able to get disable functionality working after pulling latest. Let's address the greying out or removal of "edit" in this PR @TheAndrewJackson

@TheAndrewJackson
Copy link
Contributor Author

@eastandwestwind I went ahead and removed the "edit" button 👍

eastandwestwind
eastandwestwind approved these changes Jun 23, 2022
View reviewed changes
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

LGTM! Just noted an idea for future ticket / refactor work, and additionally pushed out a small change to remove some unused code. Thanks for the hard work on this @TheAndrewJackson !

clients/admin-ui/src/features/datastore-connections/datastore-connection.slice.ts
system_type,
disabled_status,
}: Partial<DatastoreConnectionParams>): string {
let queryString = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, could be refactored to push each query prop / val to an array of maps, then concatenate with & at the end. This would avoid all the checks for queryString.

@eastandwestwind eastandwestwind merged commit 7c3bf98 into main Jun 23, 2022
@eastandwestwind eastandwestwind deleted the 669_datastore_connection_filtering branch June 23, 2022 01:42
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
@TheAndrewJackson @eastandwestwind
Datastore Connection Filtering (#691)
478b306 
* Refactor routes into enums and create connections page

* Test switching back to double quotes

* Convert back to double quotes

* Add placeholder connection filters

* Set up api scaffolding

* Get basic grid going

* Initial grid card styling

* Fix simple eslint issues

* Add development config back in

* Finish draft of card

* Add working test button and landing page

* Add pagination and small fixes

* Fix testing issues

* Add auth tests for datastore connection page

* run formatter

* Update changelog

* update the create_test_data command to add connectionconfigs

* Disable create buttons & fix text overflow

* Update filter dropdown values

* Fix test timestamp bug

* Remove development variable

* Add working filter dropdowns

* Add outside click hook & polish things

* Fix imports

* Update changelog

* Update button hover color

* remove commented out code

* fix typo

* Remove Saas Option

* Fix welcome screen bug

* Remove edit button

* Fix lint and formatting issues

* removes commented-out code

Co-authored-by: Sean Preston <[email protected]>
Co-authored-by: eastandwestwind <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eastandwestwind eastandwestwind eastandwestwind approved these changes

Assignees

@eastandwestwind eastandwestwind

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Datastore Management] Adding filtering capabilities to landing page FRONT END
3 participants
@TheAndrewJackson @eastandwestwind @adriaaaa