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

Fix buggy order fulfillment options #748

Merged
merged 6 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion adminapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"test": "vitest --passWithNoTests --globals",
"prettier-check": "prettier --no-error-on-unmatched-pattern --check '*.{jsx,js}' './tools/*.{jsx,js}' './src/**/*.{jsx,js}'",
"prettier-fix": "prettier --no-error-on-unmatched-pattern --write '*.{jsx,js}' './tools/*.{jsx,js}' './src/**/*.{jsx,js}'",
"eslint-check": "eslint --ext .js,.jsx --no-error-on-unmatched-pattern '*.js' './tools/' './src/'",
"eslint-check": "eslint --ext .js,.jsx --max-warnings=0 --no-error-on-unmatched-pattern '*.js' './tools/' './src/'",
"eslint-fix": "eslint --ext .js,.jsx --no-error-on-unmatched-pattern --fix '*.js' './tools/' './src/'"
},
"eslintConfig": {
Expand Down
1 change: 1 addition & 0 deletions lib/suma/api/commerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ class DetailedOrderHistoryEntity < SimpleOrderHistoryEntity
&self.delegate_to(:checkout, :cart, :offering, :fulfillment_confirmation)
expose :fulfillment_option, with: FulfillmentOptionEntity, &self.delegate_to(:checkout, :fulfillment_option)
expose :fulfillment_options_for_editing, with: FulfillmentOptionEntity
expose :fulfillment_option_editable?, as: :fulfillment_option_editable

expose :order_status
expose :can_claim?, as: :can_claim
Expand Down
9 changes: 9 additions & 0 deletions lib/suma/commerce/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,15 @@ def fulfillment_options_for_editing
return opts
end

# Return true if the order's fulfillment option is editable.
# It is editable when there are any options to choose from,
# that is not the currently chosen option.
def fulfillment_option_editable?
opts = self.fulfillment_options_for_editing
opts.delete(self.checkout.fulfillment_option)
return opts.any?
end

def apply_fulfillment_quantity_changes
self.items_and_product_inventories.each do |ci, inv|
inv.quantity_on_hand -= ci.quantity if inv.limited_quantity?
Expand Down
25 changes: 25 additions & 0 deletions spec/suma/commerce/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,31 @@
end
end

describe "fulfillment_option_editable?" do
let(:offering) { Suma::Fixtures.offering.create }
let(:order) do
checkout = Suma::Fixtures.checkout(cart: Suma::Fixtures.cart(offering:).create).completed.create
Suma::Fixtures.order(checkout:).create
end

it "returns false if there are no options in the offering" do
order.checkout.update(fulfillment_option: nil)
order.checkout.cart.offering.fulfillment_options.each(&:soft_delete)
expect(order).to_not be_fulfillment_option_editable
end

it "returns true when no option is chosen and theres at least one available" do
order.checkout.update(fulfillment_option: nil)
expect(order).to be_fulfillment_option_editable
end

it "returns true when option is chosen and there are more than one available" do
expect(order).to_not be_fulfillment_option_editable
another_option = Suma::Fixtures.offering_fulfillment_option.create(offering:)
expect(order).to be_fulfillment_option_editable
end
end

describe "fulfillment state machine" do
let(:limited_product) { Suma::Fixtures.product.limited_quantity(4, 2).create }
let(:unlimited_product) { Suma::Fixtures.product.create }
Expand Down
2 changes: 1 addition & 1 deletion webapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"serve": "vite preview",
"prettier-check": "prettier --no-error-on-unmatched-pattern --check '*.{jsx,js}' './tools/*.{jsx,js}' './src/**/*.{jsx,js}'",
"prettier-fix": "prettier --no-error-on-unmatched-pattern --write '*.{jsx,js}' './tools/*.{jsx,js}' './src/**/*.{jsx,js}'",
"eslint-check": "eslint --ext .js,.jsx --no-error-on-unmatched-pattern . './tools/' './src/'",
"eslint-check": "eslint --ext .js,.jsx --max-warnings=0 --no-error-on-unmatched-pattern . './tools/' './src/'",
"eslint-fix": "eslint --ext .js,.jsx --no-error-on-unmatched-pattern --fix . './tools/' './src/'"
},
"eslintConfig": {
Expand Down
2 changes: 1 addition & 1 deletion webapp/public/locale/en/out/strings.out.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions webapp/public/locale/en/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
"link_new_payment_or": "Or, link a new payment method to pay for this order.",
"mobility_options": "Mobility Options",
"no_cart_items": "This cart does not have any items.",
"no_option_chosen": "No option chosen",
"no_orders": "You haven’t place any orders yet.",
"no_orders_to_claim": "You have no orders to claim.",
"no_products": "There were no products found, this offering might be closed.",
Expand Down
2 changes: 1 addition & 1 deletion webapp/public/locale/es/out/strings.out.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions webapp/public/locale/es/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
"link_new_payment_or": "O vincula un nuevo método de pago para pagar este pedido.",
"mobility_options": "Opciones de movilidad",
"no_cart_items": "Este carrito no tiene ningún artículo.",
"no_option_chosen": "Ninguna opción elegida",
"no_orders": "Aún no ha realizado ningún pedido.",
"no_orders_to_claim": "No hay pedidos para reclamar.",
"no_products": "No se encontraron productos, esta oferta podría estar cerrada.",
Expand Down
36 changes: 18 additions & 18 deletions webapp/src/components/OrderDetail.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import useErrorToast from "../state/useErrorToast";
import useScreenLoader from "../state/useScreenLoader";
import useUser from "../state/useUser";
import PressAndHold from "./PressAndHold";
import clsx from "clsx";
import isEmpty from "lodash/isEmpty";
import React from "react";
import Alert from "react-bootstrap/Alert";
Expand Down Expand Up @@ -106,14 +105,14 @@ function FulfillmentOption({ order, onOrderUpdated }) {

if (isEmpty(order.fulfillmentOptionsForEditing)) {
if (!order.fulfillmentOption) {
// No options, and nothing is selected, so nothing to show
return null;
}
// No options, but something is selected, so show it
return (
<div>
<h6 className="fw-bold">{order.fulfillmentConfirmation}</h6>
{order.fulfillmentOption.description && (
<span>{order.fulfillmentOption.description}</span>
)}
<span>{order.fulfillmentOption.description}</span>
</div>
);
}
Expand All @@ -123,21 +122,22 @@ function FulfillmentOption({ order, onOrderUpdated }) {
<span>
<h6 className="fw-bold lh-lg">
{order.fulfillmentConfirmation}
<Button
variant="link"
className={clsx(
"p-0 ms-2",
order.fulfillmentOptionsForEditing.length === 1 && "opacity-0 disabled"
)}
onClick={() => {
setOptionId(order.fulfillmentOption.id);
editing.turnOn();
}}
>
<i className="bi bi-pencil-fill"></i>
</Button>
{order.fulfillmentOptionEditable && (
<Button
variant="link"
className="p-0 ms-2"
onClick={() => {
setOptionId(order.fulfillmentOption?.id || 0);
editing.turnOn();
}}
>
<i className="bi bi-pencil-fill" />
</Button>
)}
</h6>
{order.fulfillmentOption.description}
{order.fulfillmentOption?.description || (
<span className="text-secondary">{t("food:no_option_chosen")}</span>
)}
</span>
);
}
Expand Down
Loading