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

🐞 Marketplace: Buying Products: Guest Shopper is shown a 404 after Checkout` #1657

Closed
Tracked by #1326 ...
anaulin opened this issue Jul 13, 2023 · 2 comments
Closed
Tracked by #1326 ...
Assignees
Labels
🐞 bug Something isn't working

Comments

@anaulin
Copy link
Member

anaulin commented Jul 13, 2023

After a successful purchase from the Marketplace, a shopper got redirected to their order page (https://marketplace.piikup.com/spaces/piikup/rooms/crumbleandwhisk/marketplaces/4cad5db3-562a-4630-b31d-fc6a253ea5a9/orders/45f6c244-4e32-43c1-aa4b-8e002e278d81?stripe_session_id=cs_live_b1GZZOiJS3UGIqMKQdk5PMAPiwtdA3Me3ZXIYkePdKHoGnGYXRo4mJdtMr), but this page is only visible to logged-in users, so instead they got the standard Rails 404 page.

We should make it so that a successful order confirmation page is visible even if the user is logged-out of Convene, which is going to be the most common case. One way to do this would be to create a separate "order confirmed" page that is visible for logged out users and directs users to log into Convene to see their complete order history.

@anaulin anaulin added the 🐞 bug Something isn't working label Jul 13, 2023
@zspencer zspencer changed the title 🐞 Marketplace: guest shopper lands on a 404 after checkout 🐞 Marketplace: Guest Shopper is shown a 404 after Checkout Jul 13, 2023
@zspencer zspencer self-assigned this Jul 14, 2023
@zspencer
Copy link
Member

I'm planning to work on this either between now and Wednesday or on Wednesday.

zspencer added a commit that referenced this issue Jul 20, 2023
- #1657

This allows `Orders` placed by `Guests` to be seen by any `Guest` for 1
week; after which they will 404.

There's probably something smerter to do around the
`Marketplace::Policy` to use the `Shopper`, which *is* both present and
persisted in all cases...
zspencer added a commit that referenced this issue Jul 20, 2023
- #1326
- #1657

This gives us a system test for guests buying products

As we add more things we want to test; we should be able to expand this
one test; or re-use some of the helper methods to try different
permutations.

Bear in mind, CI will need a secret token for stripe; which I should be
able to make go.
zspencer added a commit that referenced this issue Jul 20, 2023
- #1326
- #1657

This gives us a system test for guests buying products

As we add more things we want to test; we should be able to expand this
one test; or re-use some of the helper methods to try different
permutations.

Bear in mind, CI will need a secret token for stripe; which I should be
able to make go.
zspencer added a commit that referenced this issue Jul 21, 2023
- #1657

This adjusts the `OrdersController` to use the `Shopper` for the
`OrderPolicy::Scope`

It's a bit of a hack; but also appears to fix the bug...
zspencer added a commit that referenced this issue Jul 21, 2023
- #1657

This adjusts the `OrdersController` to use the `Shopper` for the
`OrderPolicy::Scope`

It's a bit of a hack; but also appears to fix the bug...
zspencer added a commit that referenced this issue Jul 21, 2023
- #1657

This adjusts the `OrdersController` to use the `Shopper` for the
`OrderPolicy::Scope`

It's a bit of a hack; but also appears to fix the bug...
zspencer added a commit that referenced this issue Jul 22, 2023
- #1657

This adjusts the `OrdersController` to use the `Shopper` for the
`OrderPolicy::Scope`

It's a bit of a hack; but also appears to fix the bug...
zspencer added a commit that referenced this issue Jul 22, 2023
- #1326
- #1657

This gives us a system test for guests buying products

As we add more things we want to test; we should be able to expand this
one test; or re-use some of the helper methods to try different
permutations.

Bear in mind, CI will need a secret token for stripe; which I should be
able to make go.
zspencer added a commit that referenced this issue Jul 22, 2023
* 🥗 `Marketplace`: Buying `Products`: System Test

- #1326
- #1657

This gives us a system test for guests buying products

As we add more things we want to test; we should be able to expand this
one test; or re-use some of the helper methods to try different
permutations.

Bear in mind, CI will need a secret token for stripe; which I should be
able to make go.

* Listen better to the Stripe

* Install stripe cli

* Raising is hard

* Don't expose the stripe api key on failure

* Fix test that got caught in the cross-fire

* Update spec/furniture/marketplace/buying_products_system_spec.rb

Co-authored-by: Ana Ulin <ana@ulin.org>

* Pull `stripe_listen` to it's own thing

---------

Co-authored-by: Ana Ulin <ana@ulin.org>
zspencer added a commit that referenced this issue Jul 23, 2023
- #1657

It felt really weird to me to have a module calling `let` from within
the `app` directory; since that's very much an `rspec`-thing.

This pulls it out to it's own place; ensures `solargraph` can find the
class definition, comments it, and uses it almost everywhere
`let(:marketplace)` is used.

Again, not sure it's a great / good idea...
zspencer added a commit that referenced this issue Jul 23, 2023
- #1657

This is a _very weird_ PR; but I think gives us a better interface for
building out features in the `Marketplace` because now a `Person` and
`Guest` can know about their `Shopper`.

There's some stuff that gave me ?!?! while making this work, which I'll
do my best to enumerate so we can at least discuss during the review
process and reference them in some future git-blame

1. The `Marketplace::CurrentPerson` module feels very weird to me.
2. The `Marketplace::Policy::SpecFactories` really does not want to live
   where it lives IMO; and it feels like a bit of an obstraction... But
   it was also very convenient that all the specs that used it were
   switched to a `Marketplace::Person` and `Marketplace::Guest` at the
   same time...
zspencer added a commit that referenced this issue Jul 23, 2023
- #1657

It felt really weird to me to have a module calling `let` from within
the `app` directory; since that's very much an `rspec`-thing.

This pulls it out to it's own place; ensures `solargraph` can find the
class definition, comments it, and uses it almost everywhere
`let(:marketplace)` is used.

Again, not sure it's a great / good idea...
zspencer added a commit that referenced this issue Jul 23, 2023
- #1657

It felt really weird to me to have a module calling `let` from within
the `app` directory; since that's very much an `rspec`-thing.

This pulls it out to it's own place; ensures `solargraph` can find the
class definition, comments it, and uses it almost everywhere
`let(:marketplace)` is used.

Again, not sure it's a great / good idea...
@zspencer
Copy link
Member

I have been able to confirm this through the automated system test; are there any @zinc-collective/contributors interested in double-checking that the fix is "good enough?"

If so, assign this issue to yourself and run through placing an order! (Note: This is... not trivial locally due to the need to be connected to Stripe, so it may not be an ideal thing to tackel unless you want to work through that molasses or have already done so)

If not, I'll close this out next time I'm looking at my assigned issues.

zspencer added a commit that referenced this issue Jul 27, 2023
- #1657

This is a _very weird_ PR; but I think gives us a better interface for
building out features in the `Marketplace` because now a `Person` and
`Guest` can know about their `Shopper`.

There's some stuff that gave me ?!?! while making this work, which I'll
do my best to enumerate so we can at least discuss during the review
process and reference them in some future git-blame

1. The `Marketplace::CurrentPerson` module feels very weird to me.
2. The `Marketplace::Policy::SpecFactories` really does not want to live
   where it lives IMO; and it feels like a bit of an obstraction... But
   it was also very convenient that all the specs that used it were
   switched to a `Marketplace::Person` and `Marketplace::Guest` at the
   same time...
@anaulin anaulin closed this as completed Aug 10, 2023
@zspencer zspencer changed the title 🐞 Marketplace: Guest Shopper is shown a 404 after Checkout 🐞 Marketplace: Buying Products: Guest Shopper is shown a 404 after Checkout` Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants