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

Document that only one Order type should be used in a Salespoint application #415

Open
Deric-W opened this issue Nov 23, 2022 · 2 comments
Labels
in: documentation Reference documentation, Javadoc, website module: orders Orders type: enhancement Improvements and new features

Comments

@Deric-W
Copy link

Deric-W commented Nov 23, 2022

Hi,
I tried creating two Subclasses of Order and storing them in separate OrderManagement<T> with specific types.
Calling OrderManagement<T>::findAll(Pageable.unpaged()) yields a Page containing all orders, regardless of type or order management.

Is this use case not supported?

ProductOrder.java
CustomerOrder.java
OrderDataInitializer.java

@martinmo
Copy link
Contributor

Hi, thanks for the report! My understanding is that the intention was to support this use case and provide type-safe access to Order subclasses via findAll(…), get(…) and so on, so this seems to be a bug.

As a workaround (and to avoid filtering in Java) you can declare custom repository interfaces with that findAll(…) method for each of your Order subclasses that extends Spring's Repository as shown in the Salespoint reference documentation. If you need a method query method several times, you can create a base interface and annotate it with @NoReposityBean.

@NoRepositoryBean // suppress creation of bean, because T is unknown here
public interface BaseOrderRepository<T extends Order> extends Repository<T, OrderIdentifier> {
	Page<T> findAll(Pageable pageable);
}

public interface ProductOrderRepository extends BaseOrderRepository<ProductOrder> {

}

public interface CustomerOrderRepository extends BaseOrderRepository<CustomerOrder> {

}

@martinmo
Copy link
Contributor

… the intention was to support this use case and provide type-safe access to Order subclasses via findAll(…), get(…) and so on, so this seems to be a bug …

I need to correct my statement after talking to @odrotbohm – supporting this use case was not intended. In Salespoint, you must always use either the stock Order or one subclass of Order. Mixing them or with more than one subclass, a lot of assumptions in OrderManagement will break and type-safety issues arise.

There are a lot of reasons that this use case isn't supported. While in theory, it would be possible to support this in the framework, it would make the standard case – using just the stock Order – a lot harder to use. Another one is that most often, you can create a simpler, equivalent design that doesn't depend on multiple Order subclasses.

For example, in your case, you could either

  1. use only the stock Order and rely on the orders association in your Customer class, or
  2. stick with just CustomerOrder with the optional Customer attribute, where absence means it is a product order. In this case, the orders association in Customer is redundant.

However, this can be improved in the documentation because this design constraint isn't mentioned.

@martinmo martinmo self-assigned this Nov 24, 2022
@martinmo martinmo added type: enhancement Improvements and new features in: documentation Reference documentation, Javadoc, website and removed type: bug Bugs labels Nov 24, 2022
@martinmo martinmo changed the title OrderManagement<T>::findAll breaks type safety Document that only one Order type should be used in a Salespoint application Nov 24, 2022
@martinmo martinmo removed their assignment Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: documentation Reference documentation, Javadoc, website module: orders Orders type: enhancement Improvements and new features
Projects
None yet
Development

No branches or pull requests

2 participants