Add the ability to add an address to a store - #6649
sascha-karnatz wants to merge 4 commits into
Conversation
github-actions
Bot
added
changelog:solidus_api
3d64213 to
c36c379
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6649 +/- ##
==========================================
+ Coverage 92.37% 92.38% +0.01%
==========================================
Files 1055 1055
Lines 21383 21400 +17
==========================================
+ Hits 19752 19770 +18
+ Misses 1631 1630 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c6cd73c to
667475c
Compare
| # Addresses are immutable, so a changed address is stored as a new record. | ||
| # Blank attributes are ignored as long as the store has no address. | ||
| # | ||
| # @param attributes [Hash] the address attributes | ||
| def address_attributes=(attributes) | ||
| attributes = attributes.to_h.stringify_keys | ||
| return if address.nil? && attributes.except("country_id", "state_id", "reverse_charge_status").values.all?(&:blank?) | ||
|
|
||
| self.address = Spree::Address.immutable_merge(address, attributes) | ||
| end |
There was a problem hiding this comment.
The #address_attributes= method on Spree::CreditCard is just this:
def address_attributes=(attributes)
self.address = Spree::Address.immutable_merge(address, attributes)
endThe association looks very similar here, so I am uncertain why this method on the store model needs to be more complex than that one.
What is the need to treat attribute keys as strings and treat some attributes exceptionally?
There was a problem hiding this comment.
Because the address can be optional and it will be shown in the admin/backend form. If the user is submitting the form, the server would try to create an address, because the address has always some preselected fields. I changed the setter to make it better readable.
I also considered to add this change into the controller, but I would have change both controllers for admin, and backend and than have to strip address parameter, if the address has only a country and state.
There was a problem hiding this comment.
Thanks for explaining! And I think that the changes you pushed today make this a bit clearer for me.
I also considered to add this change into the controller, but I would have change both controllers for admin, and backend and than have to strip address parameter, if the address has only a country and state.
I think that the special treatments of the attributes have more to do with the view and controller logic than the model logic. I feel that the model should only be concerned with the true address and not the presentation layer of the admin interface.
So, while it's code that would need to be duplicated across the backend and admin libraries, I think that's still worth doing.
There was a problem hiding this comment.
I'm not an AI, "you're right". I'm going to update the PR.
667475c to
b73190c
Compare
Stores had no way to record their own postal address. Invoices usually have to show the seller's address, and so can packing slips, order emails, return labels and the storefront footer.
Render the address field if an address is connected to the store. The index and show views share a new `_store` partial, and the OpenAPI spec documents both the new response field and the new input field.
Add the address form to the stores edit view. Add only basic address information including vat_id to reduce the number of form fields and make the address panel a bit more compact. Email and phone number are normally not necessary on invoices or packaging slips. The phone number is only available if the address_requires_phone is enabled.
Lets backend users set the store address for invoices, matching the new admin. Both forms hide email and the second street line, and hide phone unless `address_requires_phone` is enabled. The shared address partial now accepts an optional `excludes` local. This change was necessary to reduce the number of fields.
b73190c to
8c1734d
Compare
adammathys
left a comment
adammathys
left a comment
There was a problem hiding this comment.
Looks reasonable to me! 👍🏻
4 participants
Summary
Adds an optional address to
Spree::Store, mainly so invoices can show the seller's address. It is maybe also useful for packing slips, emails, return labels and storefront footers.This change partially fixes that issue #6112. A store would have through the address a connected VAT Id.
Screenshots
Admin:
Backend:
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: