-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Initial orders documentation #2498
Initial orders documentation #2498
Conversation
4a86d30
to
5ad5365
Compare
5ad5365
to
70b2e34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the overview section:
currency: The currency for this order. Determined by the
last_ip_address: The last IP address used to update this order in the frontend.
created_by_id: The ID of object that created this order. Spree::Config[:currency] value that was set at the time of order.
Should be:
currency: The currency for this order. Determined by the Spree::Config[:currency] value that was set at the time of order.
last_ip_address: The last IP address used to update this order in the frontend.
created_by_id: The ID of object that created this order.
70b2e34
to
d5deacb
Compare
@avremel Thanks for catching that error. Fixed! |
d5deacb
to
76c7529
Compare
- Add article about display_total methods and other helpful methods. - Add summaries of models that are related to the Spree::Order model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left a couple of questions
its [`format`][ruby-money-format] method: | ||
|
||
```ruby | ||
@order.display_total.format(:with_currency => true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using the new ruby syntax here? .format(with_currency: true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
guides/orders/overview.md
Outdated
|
||
- `number`: The unique identifier for this order. It begins with the letter `R` | ||
and ends in a nine-digit number (for example, `R123456789`). This number is | ||
shown to the users, and can be used to find the order by calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe here we should link the Order::NumberGenerator
which allows to change how this number is generated. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thanks for the suggestion.
guides/orders/overview.md
Outdated
`Spree::Address` objects with billing and shipping address information. | ||
- `payment_total`: The sum of all the *finalized* payments on the order. | ||
- `shipment_state`: The current [shipment state][shipment-states] of the order. | ||
- `payment_state`: The current payment state of the order. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this state needs a link to its own documentation like shipment state
, right? I assume it's not here since the payment docs is not merged yet, what about a todo? is it possible within the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good call. Thank you!
Thanks to kennyadsl for his review.
@kennyadsl Thanks for the review! |
I ported the orders documentation over from the Spree guides. Then, I rewrote it with Solidus in mind. I also split the article into four smaller articles.
The main overview article summarizes how some other models (like
Spree::Adjustment
andSpree::Shipment
) supplement the order, but in some cases I was not able to link to additional documentation because it doesn't exist yet (or just hasn't been merged). I'll come back and do that when the time is right.This is part a larger project to improve Solidus's documentation. See this gist with the high-level table of contents. Where and how this documentation will exist is still up for discussion.