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.

Sign up for GitHub

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

Jump to bottom

🐞🔨 Marketplace: Buying Products Fix Delivery Form #1979

Merged
merged 1 commit into from
Nov 30, 2023
Merged

🐞🔨 Marketplace: Buying Products Fix Delivery Form #1979

merged 1 commit into from
Nov 30, 2023

Conversation

zspencer
Copy link
Member

@zspencer zspencer commented Nov 30, 2023 •
edited
Loading

Turns out, our Delivery form was going to a weird spot which may or may not be why we couldn't place orders on Crumble & Whisk

Anyway, now instead of posting to
/marketplace/carts/:cart_id/delivery.:delivery_id it will post to /marketplace/carts/:cart_id/delivery

@zspencer zspencer requested review from a team November 30, 2023 01:41
@zspencer
🐞🔨 Marketplace: Buying Products Fix Delivery Form
6fb47f6 
Turns out, our `Delivery` form was going to a weird spot which may or
may not be why we couldn't place orders on Crumble & Whisk

Anyway, now instead of posting to
`/marketplace/carts/:cart_id/delivery.:delivery_id` it will post to
`/marketplace/carts/:cart_id/delivery`
@zspencer zspencer force-pushed the marketplace/turbo-frames-should-not-live-in-partials-maybe branch from d227ebb to 6fb47f6 Compare November 30, 2023 02:09
anaulin
anaulin approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@anaulin anaulin left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

(I feel like we keep tripping over our location business, but I don't have a suggestion for improvement at the moment.)

@anaulin anaulin merged commit 393fa8b into main Nov 30, 2023
@anaulin anaulin deleted the marketplace/turbo-frames-should-not-live-in-partials-maybe branch November 30, 2023 02:36
@anaulin anaulin mentioned this pull request Nov 30, 2023
Closed
11 tasks
@zspencer zspencer added 🐞 bug Something isn't working 🔨 Squashed Bugs we think we fixed labels Nov 30, 2023
@zspencer
Copy link
Member Author

Sadly, this is because of how Rails presumes plural resource routing by default rather than singular. We would have had the same problem if we had used form_with(model: delivery).

We could err on the side of over-specification and always include a url: argument in form_with calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anaulin anaulin anaulin approved these changes

Assignees
No one assigned
Labels
🐞 bug Something isn't working 🔨 Squashed Bugs we think we fixed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zspencer @anaulin