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

feat: Add UI for SAML attribute mapping #4184

Merged
merged 8 commits into from
Jun 24, 2024

Conversation

novakzaballa
Copy link
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

  • Add a new tab for the SAML attribute mapping
  • Improve SAML modal

How did you test this code?

  1. Go to Organisation settings -> SAML Tab
  2. Create or select a SAML configuration
  3. Go to the Attribute Mapping tab and create a new Attribute

Copy link

vercel bot commented Jun 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 24, 2024 2:36pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 24, 2024 2:36pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Jun 24, 2024 2:36pm

@github-actions github-actions bot added front-end Issue related to the React Front End Dashboard feature New feature or request labels Jun 18, 2024
Copy link
Contributor

github-actions bot commented Jun 18, 2024

Uffizzi Preview deployment-53181 was deleted.

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Jun 18, 2024
@novakzaballa novakzaballa requested review from a team and kyle-ssg and removed request for a team June 18, 2024 19:28
@novakzaballa novakzaballa linked an issue Jun 18, 2024 that may be closed by this pull request
@matthewelwell
Copy link
Contributor

@novakzaballa I've released the SAML fix that you created to staging so this can now be tested correctly against staging, but I've found a few issues:

  1. It should be possible to add attributes as part of the creation workflow (or at least the modal shouldn't auto close)
  2. The FE is making an invalid request initially when opening the edit modal to retrieve the mappings for saml_configuration=0 (see screenshot 1 below)
  3. The UI doesn't handle long IdP attributes (see screenshot 2 below - note that this is a very valid attribute and is used in a lot of our customer's configurations)
  4. The attributes aren't editable but appear like they are (as it shows a pointer when you hover over each row in the table). I guess making them editable would be tricky given the fact that we're already in a modal, so perhaps we should just remove the pointer on hover?
image image

@novakzaballa
Copy link
Contributor Author

  1. It should be possible to add attributes as part of the creation workflow (or at least the modal shouldn't auto close)

Now the modal does not close when the configuration is created, and the SAML attribute tab displays immediately.

  1. The FE is making an invalid request initially when opening the edit modal to retrieve the mappings for saml_configuration=0 (see screenshot 1 below)

Corrected

  1. The UI doesn't handle long IdP attributes (see screenshot 2 below - note that this is a very valid attribute and is used in a lot of our customer's configurations)

Done
Screenshot 2024-06-20 at 3 57 44 PM

  1. The attributes aren't editable but appear like they are (as it shows a pointer when you hover over each row in the table). I guess making them editable would be tricky given the fact that we're already in a modal, so perhaps we should just remove the pointer on hover?

Removed.

@matthewelwell
Copy link
Contributor

The overflow issue still isn't resolved. The key thing we need to solve it for is not multiple words, but a single long URL. The test case you used is not really valid.

image

@matthewelwell matthewelwell requested a review from a team as a code owner June 24, 2024 10:31
@matthewelwell
Copy link
Contributor

I've fixed this here but I'm seeing weird behaviour on the tooltip. It looks like the container for the tooltip itself isn't wide enough for the string but I have no idea how to resolve that...

image

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Jun 24, 2024
@novakzaballa
Copy link
Contributor Author

I've fixed this but I'm seeing weird behaviour on the tooltip. It looks like the container for the tooltip itself isn't wide enough for the string but I have no idea how to resolve that...

Screenshot 2024-06-24 at 10 34 43 AM

Done

@matthewelwell matthewelwell added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit 318fb85 Jun 24, 2024
27 checks passed
@matthewelwell matthewelwell deleted the feat/add-ui-for-saml-attribute-mapping branch June 24, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request front-end Issue related to the React Front End Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add UI for configuring SAML attribute mappings
2 participants