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.

Sign up for GitHub

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

Jump to bottom

fix: Frontend error when creating SAML configuration if API URL is relative #4751

Merged
merged 1 commit into from
Oct 21, 2024
Merged

fix: Frontend error when creating SAML configuration if API URL is relative #4751

merged 1 commit into from
Oct 21, 2024

Conversation

rolodato
Copy link
Member

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

  • 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

If Project.api points to a relative URL such as /api/v1, clicking on "Create SAML Configuration" or opening an existing SAML configuration results in an error:

Uncaught TypeError: URL constructor: /api/v1/ is not a valid URL.
    CreateSAML CreateSAML.tsx:58

This does not happen with the default npm run dev because we're setting an absolute URL of http://localhost:8000/api/v1/. To reproduce this bug locally, have the FE proxy requests to the BE by running like this:

FLAGSMITH_PROXY_API_URL=http://127.0.0.1:8000 npm run dev

How did you test this code?

Manually - using flagsmith/flagsmith-private-cloud as the API, try to create a SAML configuration or edit an existing one.

@rolodato rolodato requested a review from a team as a code owner October 18, 2024 21:33
@rolodato rolodato requested review from kyle-ssg and removed request for a team October 18, 2024 21:33
@vercel Vercel
Copy link

vercel bot commented Oct 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 Oct 18, 2024 9:33pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2024 9:33pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Oct 18, 2024 9:33pm

@github-actions github-actions bot added front-end Issue related to the React Front End Dashboard fix labels Oct 18, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 18, 2024 •
edited
Loading

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-4751 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-4751 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api:pr-4751 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4751 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith:pr-4751 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-frontend:pr-4751 Finished ✅ Results ✅

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 18, 2024 •
edited
Loading

Uffizzi Preview deployment-57435 was deleted.

khvn26
khvn26 approved these changes Oct 21, 2024
View reviewed changes
Copy link
Member

@khvn26 khvn26 left a comment

Choose a reason for hiding this comment

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

LGTM

@rolodato rolodato added this pull request to the merge queue Oct 21, 2024
Merged via the queue into main with commit df1b84e Oct 21, 2024
31 checks passed
@rolodato rolodato deleted the fix/create-saml-relative-api-url branch October 21, 2024 12:16
@flagsmithdev flagsmithdev mentioned this pull request Oct 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khvn26 khvn26 khvn26 approved these changes

@kyle-ssg kyle-ssg Awaiting requested review from kyle-ssg kyle-ssg is a code owner automatically assigned from Flagsmith/flagsmith-front-end

Assignees
No one assigned
Labels
fix front-end Issue related to the React Front End Dashboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rolodato @khvn26