-
-
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
[Admin] Construct base components for order creation in admin interface #5434
[Admin] Construct base components for order creation in admin interface #5434
Conversation
4b4579d
to
1ede2a5
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.
I think you can squash the first commit into the last one, after all the newly created component is not very interesting by itself.
def new | ||
@order = Spree::Order.new( | ||
{ | ||
created_by_id: spree_current_user.try(:id), |
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.
spree_current_user
comes from one of many possible authentication methods, here it's better to use the internal method for getting the admin user. Also no nil checks are needed as from we no longer support not having an authenticated user in the admin.
created_by: current_solidus_admin_user
admin/spec/components/solidus_admin/orders/new/component_spec.rb
Outdated
Show resolved
Hide resolved
1ede2a5
to
2b8360a
Compare
{ | ||
created_by: current_solidus_admin_user, | ||
frontend_viewable: false, | ||
store_id: current_store.try(:id) | ||
} |
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.
nit: don't need to wrap in curly braces, I would even expected rubocop to complain about this 🤷♂️
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.
Good catch! You're right, it's a copy-paste leftover from the old backend orders controller 😅 Will clean that up, thanks!
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.
I left a final nit, but otherwise LGTM 👍👍👍
This commit introduces a new `orders/new` component that includes a header, providing users with 'Save' and 'Discard' button options. Although the UI elements have been integrated, the functionalities for these buttons will be implemented in subsequent iterations.
2b8360a
to
b8f3a4d
Compare
Summary
This PR lays the foundation for the
orders/new
component and integrates basic UI elements withinsolidus_admin
.orders/new
componentSave
andDiscard
buttonsChecklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: