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

Fullpage redirect in embedded apps using shopify_app/shared/redirect stopped working #1728

Open
fluke opened this issue Sep 28, 2023 · 3 comments

Comments

@fluke
Copy link

fluke commented Sep 28, 2023

Issue summary

I have been using the shopify_app/shared/redirect page to do page redirects in my app. The way that it was working was that if you land on the root page, if you haven't signed up for a plan you get redirected in a before_action to the plans page. However as of yesterday (as far as I can confirm) the shopify_app/shared/redirect template renders on the page but nothing happens. I can confirm there is no change from my side but appears to be a change from the Shopify side.

  • shopify_api version: 12.5.0
  • shopify_app version: 21.4.0
  • Ruby version: 3.1.2
  # This is inspired by the fullpage_redirect_to used in different places in shopify_app
  def embedded_redirect_to(path)
    if ShopifyApp.configuration.embedded_app?
      raise ::ShopifyApp::ShopifyDomainNotFound if current_shopify_domain.blank?
      raise ::ShopifyApp::ShopifyHostNotFound if params[:host].blank?

      url = ShopifyAPI::Auth.embedded_app_url(params[:host]) + path

      render("shopify_app/shared/redirect", layout: false,
        locals: { url: url, current_shopify_domain: current_shopify_domain })
    else
      redirect_to(path)
    end
  end

Expected behavior

The view renders but then quickly redirects to the relevant page.

Actual behavior

The view renders but redirection doesn't happen.

Screenshot 2023-09-28 at 11 30 54 AM

Steps to reproduce the problem

  1. Redirect to another page using shopify_app/shared/redirect like mentioned in the method above in a before_action when loading any page.

Hypothesis

Something has changed from Shopify or the App Bridge utils used to redirect. I have a temporary solution where I load my React page and then trigger a redirect using the below. So maybe the solution is to just change the redirection solution.

export const redirectTo = (path) => {
  const app = createShopifyApp()
  const redirect = Redirect.create(app)
  redirect.dispatch(Redirect.Action.APP, path)
}
@fluke
Copy link
Author

fluke commented Sep 28, 2023

One thing I noticed. A lot of the times when it gets redirected to the shared view Shopify won't even render the app for some reason. In those cases the Redirect appears to work, the URL changes in the top bar but nothing will render. Again points to something being wrong with the AppBridge redirect

Screenshot 2023-09-28 at 12 57 50 PM

@fluke
Copy link
Author

fluke commented Sep 28, 2023

Okay looks like I've found the issue. Remote redirects back into the same embedded app are not working when they used to work before. Haven't checked if it works for fullpage_redirect_to so I'm not clear if the remote redirect is not working or not working for this use case. If I change this line: https://github.com/Shopify/shopify_app/blob/v21.4.0/app/assets/javascripts/shopify_app/app_bridge_redirect.js#L18

To: Redirect.create(app).dispatch(Redirect.Action.APP, path);

It works. So for now I will make a separate shopify_app/shared/internal_redirect for my app. If it makes sense to provide this as a general solution I can make a PR.

@fluke
Copy link
Author

fluke commented Sep 28, 2023

Putting down how I'm fixing it for my app.

// shopify_internal_redirect.js
(function(window) {
  function appBridgeInternalRedirect(path) {
    var AppBridge = window['app-bridge'];
    var createApp = AppBridge.default;
    var Redirect = AppBridge.actions.Redirect;
    var shopifyData = document.body.dataset;

    var app = createApp({
      apiKey: shopifyData.apiKey,
      host: shopifyData.host,
    });

    Redirect.create(app).dispatch(Redirect.Action.APP, path);
  }

  window.appBridgeInternalRedirect = appBridgeInternalRedirect;
})(window);

(function () {
  function redirect() {
    var redirectTargetElement = document.getElementById("redirection-target");

    if (!redirectTargetElement) {
      return;
    }

    var targetInfo = JSON.parse(redirectTargetElement.dataset.target);

    var appBridgeUtils = window['app-bridge'].utilities;

    if (appBridgeUtils.isShopifyEmbedded()) {
      window.appBridgeInternalRedirect(targetInfo.path);
    } else {
      window.top.location.href = targetInfo.path;
    }
  }

  document.addEventListener("DOMContentLoaded", redirect);

  // In the turbolinks context, neither DOMContentLoaded nor turbolinks:load
  // consistently fires. This ensures that we at least attempt to fire in the
  // turbolinks situation as well.
  redirect();
})();
<!-- shopify_app/shared/internal_redirect.html.erb -->
<!DOCTYPE html>
<html lang="<%= I18n.locale %>">
<head>
  <meta charset="utf-8" />
  <meta name="viewport" content="width=device-width, initial-scale=1" />
  <base target="_top">
  <title>Redirecting…</title>
  <script src="https://unpkg.com/@shopify/[email protected]"></script> <!--  Loading latest app-bridge -->
  <%= javascript_pack_tag('shopify_internal_redirect', crossorigin: 'anonymous', integrity: true) %>
</head>
<body data-api-key="<%= ShopifyApp.configuration.api_key %>" data-shop-origin="<%= current_shopify_domain %>" data-host="<%= params[:host] %>" >
  <%=
  content_tag(:div, nil,
    id: 'redirection-target',
    data: {
      target: {
        myshopifyUrl: "https://#{current_shopify_domain}",
        path: path,
      },
    },
  )
  %>
  <h1>Redirecting...</h1>
</body>
</html>
# embedded_controller.rb
def embedded_redirect_to(path)
    if ShopifyApp.configuration.embedded_app?
      raise ::ShopifyApp::ShopifyDomainNotFound if current_shopify_domain.blank?
      raise ::ShopifyApp::ShopifyHostNotFound if params[:host].blank?

      render(
        "shopify_app/shared/internal_redirect",
        layout: false,
        locals: {
          path: path,
          current_shopify_domain: current_shopify_domain
        }
      )
    else
      redirect_to(path)
    end
  end

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

No branches or pull requests

1 participant