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

Support Unified Admin #1658

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Support Unified Admin #1658

merged 1 commit into from
Mar 21, 2023

Conversation

klenotiw
Copy link
Contributor

@klenotiw klenotiw commented Feb 28, 2023

What this PR does

Installs with the unified admin url were not working. This PR recognizes if a unified admin url was used and converts it to the old url format. This is a bandaid right now since core doesn't support the authorize routes with the new format.

closes #1660

Reviewer's guide to testing

Try to install an app using a unified admin store url.

Things to focus on

  1. Is unified_admin?(uri) a good way to determine if a url is unified admin?

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@klenotiw klenotiw changed the title investigating stuff Add Unified Admin Feb 28, 2023
@klenotiw klenotiw changed the title Add Unified Admin Support Unified Admin Mar 6, 2023
@klenotiw klenotiw marked this pull request as ready for review March 6, 2023 14:19
@klenotiw
Copy link
Contributor Author

klenotiw commented Mar 6, 2023

I wasn't able to get this to work with spin domains yet. Working on the spin fix now, but I imagine it will be a very similar approach. Or we can ship this now and work on a spin fix later.

Copy link
Contributor

@nelsonwittwer nelsonwittwer left a comment

Choose a reason for hiding this comment

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

Nice one! I think it's worth spending time on spin domains before shipping but this looks great! 👏 👏 👏

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.

Unified Admin URL not Working
2 participants