Add ShopHost concern for fetching and saving host

[kirillplatonov]

The problem

Right now in AppBridge 2.0 apps, it's not possible to open any app link in a new tab (it won't load without host param). The only workaround is to pass host to every link in the app which is pretty hard to manage.

What this PR does

This PR introduces ShopifyApp::ShopHost concern which handles host fetching and caches it in cookies. This allows avoiding passing host param manually to every link. I found this concern very convenient and use it in all of my apps.

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.

[kirillplatonov]

cc @gonzaloriestra

[kirillplatonov]

Hey @gonzaloriestra @NabeelAhsen!
Any chance this PR could be merged? Happy to make adjustments if needed.
Right now opening any links in App Bridge 2.0 apps in the new tab is broken which is not great UX for merchants.

[CodeSchneider]

Hi @kirillplatonov I recently posted an issue for this gem. Do you think adding the host param would solve our problem. Currently in our Rails controller the redirect logic looks like:

fullpage_redirect_to charge.confirmation_url

Are you proposing something like this?

fullpage_redirect_to charge.confirmation_url + 'host=' + (@shop_domain + '/admin')

I'm a little confused where to add the host param, does it need to be base64 encoded etc.

Thanks for your help.

[gonzaloriestra]

Thanks a lot for your contribution and sorry for the late review!

It looks good to me, although I'd like to confirm the approach with @mkevinosullivan @paulomarg or @hannachen as I think they've been working more closely with this.

[kirillplatonov]

@gonzaloriestra sounds good. Resolved conflicts in this PR so it's ready to merge.

[paulomarg]

Overall this makes a lot of sense to me! I think making this a concern makes it a lot easier to add other controllers.

Just a couple of thoughts that came to me:

1. Does saving the host value to a cookie help here?
   • In embedded apps, we can't guarantee that the cookie will be saved, since it'll be a 3rd party cookie and browsers are making it harder to use those.
   • In non-embedded apps, the cookies would work but we won't be able to use App Bridge, so we won't really need the host value.
2. It'd be nice to have just a couple of unit tests for this to make sure we're grabbing the host value in the concern.

Thank you for this!

[hoppergee]

Inspired by this PR, I came up with another way to fetch host without cookies. And it works well with an app on hand. #1403

[flavio-b]

Right now, AppBridge JWTs have a payload with lots of info. Wouldn't it make more sense to include the host parameter in this payload too, then build a backend helper to access it everywhere? @hoppergee, what do you think?

[github-actions]

This pull request is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

[github-actions]

We are closing this pull request because it has been inactive for a few months.
This probably means that it is not reproducible or it has been fixed in a newer version.
If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this pull request with the latest stable version, please reopen using the pull request template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!