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

Call empty only on incomplete orders #5827

Merged

Conversation

nvandoorn
Copy link
Contributor

Summary

Fixes #4438. This change is based on #5418 from @nirnaeth.

There are two notable changes from #5418:

  1. Call order.incomplete? instead of order.shipped? as suggested by @kennyadsl
  2. Updated the new feature spec to make it pass.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@nvandoorn nvandoorn requested a review from a team as a code owner August 14, 2024 21:20
@nvandoorn nvandoorn changed the title call empty only on incomplete orders Call empty only on incomplete orders Aug 14, 2024
@github-actions github-actions bot added changelog:solidus_api Changes to the solidus_api gem changelog:solidus_backend Changes to the solidus_backend gem labels Aug 14, 2024
@nvandoorn
Copy link
Contributor Author

Looks like we have a few spec failures that we're currently addressing.

@nvandoorn nvandoorn marked this pull request as draft August 14, 2024 21:57
end

expect(page.current_path).to eq(spree.cart_admin_order_path(order))
expect(page).not_to have_content(order.line_items.first.variant.sku)
Copy link
Member

Choose a reason for hiding this comment

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

This can lead to false positives. Can we expect an empty order message instead?

Suggested change
expect(page).not_to have_content(order.line_items.first.variant.sku)
expect(page).to have_content I18n.t('spree.your_order_is_empty_add_product')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @tvdeyen. I ended up re-writting most of this spec in the latest revision, so let me know what you think :)

@nvandoorn nvandoorn force-pushed the call-empty-only-on-incomplete-orders branch from 967b064 to e96f358 Compare September 12, 2024 18:00
Code fails with a 500 on admin because of the order shipping status.
The change triggers @order.empty! only when the order is incomplete.

Co-authored-by: An Stewart <[email protected]>
Co-authored-by: Kendra Chateau <[email protected]>
@nvandoorn nvandoorn force-pushed the call-empty-only-on-incomplete-orders branch from e96f358 to c4fd1df Compare September 12, 2024 18:02
@nvandoorn nvandoorn marked this pull request as ready for review September 16, 2024 18:41
@MadelineCollier MadelineCollier merged commit 72920b8 into solidusio:main Sep 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_api Changes to the solidus_api gem changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API] empty cart endpoint raises 500 error with shipped order
5 participants