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

Revert the recent changes as they are breaking the install. #15

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

Kyon147
Copy link
Owner

@Kyon147 Kyon147 commented Dec 16, 2022

No description provided.

@Kyon147 Kyon147 merged commit ded9fb5 into master Dec 16, 2022
@rvibit
Copy link
Contributor

rvibit commented Dec 16, 2022

@Kyon147 what are the things it is breaking? I installed version 17.3.3 yesterday and one issue i noticed about the shop param is not being passed to the billable modal. are there any other things that are broken?

@Kyon147
Copy link
Owner Author

Kyon147 commented Dec 16, 2022

I've tested the billing and it works as expected.

Are you making sure you are passing the shop, like so?

<p><a href="{{ route('billing', ['plan' => 2, 'shop' => Auth::user()->name]) }}">Upgrade</a></p>

@rvibit
Copy link
Contributor

rvibit commented Dec 16, 2022

I've tested the billing and it works as expected.

Are you making sure you are passing the shop, like so?

<p><a href="{{ route('billing', ['plan' => 2, 'shop' => Auth::user()->name]) }}">Upgrade</a></p>

Yes, I have already fixed that issue by passing the shop param manually. I just wanted to know what other issues you are facing so you reverted the changes?

@Kyon147
Copy link
Owner Author

Kyon147 commented Dec 16, 2022

There were two recent PR's by another person which added the host down into the billing etc but it seemed it could be causing some issues on new installs.

To keep things clean, I reverted back to before those PRs were added in, and res-tested and the package is working as expected.

@rvibit
Copy link
Contributor

rvibit commented Dec 16, 2022

There were two recent PR's by another person which added the host down into the billing etc but it seemed it could be causing some issues on new installs.

To keep things clean, I reverted back to before those PRs were added in, and res-tested and the package is working as expected.

ok got it, thanks for the update.

@jackwefullfill
Copy link

hi @Kyon147, this means that #1 and #2 are not valid right? and we are still not able to load the app inside admin.shopify.com?

@Kyon147
Copy link
Owner Author

Kyon147 commented Dec 20, 2022

We need to remove the shopOrigin from the appBridge set up as the main concern. Then it just needs to be tested on an admin.shopify domain.

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.

None yet

3 participants