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

Received state for orders #779

Open
lentschi opened this issue Oct 17, 2020 · 4 comments
Open

Received state for orders #779

lentschi opened this issue Oct 17, 2020 · 4 comments

Comments

@lentschi
Copy link
Contributor

In order to facilitate "self-service" features such as #766 it would be nice to have a distinct "received" state in Order.

Current status
Currently it's hard to determine what (group) orders have been received - s. #766 (comment).

The order state flow is open -> finished -> closed.

Planned adaptation
The order state flow should become open -> finished -> received -> closed. All current queries checking for Order.state = 'finished' would then instead need to check for Order.state IN ('finished', 'received') (Though that may be subject to change in the future: For foodcoops in which "self service" is enabled - again see #766 - balancing may only be done for 'received' orders.)

@wvengen
Copy link
Member

wvengen commented Jan 28, 2021

Sounds good!
Not all foodcoops use the receive screen, though, so it should be possible to skip that step (finished -> closed).

@lentschi
Copy link
Contributor Author

Well this is exactly what I intended to implement. 😊 The received state is never set unless the users post the receive page.

You can still go from finished to closed in a single step. (Happens when settling the respective order for example - there is no check that the order must have been received first - see my original, self set goal: "All current queries checking for Order.state = 'finished' would then instead need to check for Order.state IN ('finished', 'received')")

Or am I missing some other foodsoft feature where this might be a problem? 🤔

@wvengen
Copy link
Member

wvengen commented Jan 30, 2021

Not that I can see. I'd be for implementing this.

It would be good to document this (e.g. in the model), including which state transitions can happen (to be sure other people realise what can be expected), and be sure to check all occurences of finished, also in plugins. And it would be good to mention it in the Changelog on the next release (for people with their own plugins).

lentschi added a commit to lentschi/foodsoft that referenced this issue Jan 31, 2021
@lentschi
Copy link
Contributor Author

It would be good to document this (e.g. in the model), including which state transitions can happen (to be sure other people realise what can be expected)

Yes, you're right! I've added it here in my PR.
Is that place fine?

... and be sure to check all occurences of finished, also in plugins.

I did. Scopes such as Order.finished already include the received state, so there's not so much that needed changing.
When going through the plugins, I found one deactivated plugin' controller where it may need to be changed:

@order_ids = Order.where(state: ['open', 'finished']).all.map(&:id)

However, I'm not sure how to display this page and the current_orders no longer seems to be working (even before my changes) - is it still maintained at all? (For example this count(distinct: true) expression no longer seems to work in Rails 5 - it produces an SQL error.)

And it would be good to mention it in the Changelog on the next release (for people with their own plugins).

Good point. Do you want me to add it directly to CHANGELOG.md with a UPCOMING heading? (I couldn't find anything about how to do this in the Developing guidelines.)

If you do this manually once you release a new version - here's what I would put in there:

Introduces the new optional order state received that is set after a user posts OrdersController#receive. The order state sequence thus becomes: open -> finished (-> received) -> closed. For most current foodsoft actions, received will be equivalent to finished. So plugin developers might need to alter their queries unless they really want to exclude orders that have been received! (The order scopes such as Order.finished already include the received state to facilitate this transition; only direct queries to state might need to be adapted.)

It's a bit verbose - so IMO you could shorten it, if you want to keep the Changelog's size low.

wvengen pushed a commit that referenced this issue Feb 3, 2021
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 5, 2021
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 5, 2021
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 5, 2021
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 5, 2021
…ers index

This slightly increases the performance as the associated order_articles
no longer need to be fetched.
paroga pushed a commit that referenced this issue Feb 5, 2021
This slightly increases the performance as the associated order_articles
no longer need to be fetched.
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 5, 2021
paroga pushed a commit that referenced this issue Feb 5, 2021
lentschi added a commit to lentschi/foodsoft that referenced this issue Feb 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants