Use OrderAddressDto for billing and shipping

[bjarnef]

I have added a OrderAddressDto to use same properties and ensure types of these are identical. It can also be considered to move the Email property to OrderAddressDto. At the moment the email update the Constants.Properties.Customer.EmailPropertyAlias. I guess this is a "email" property on the customer and not specific on the billing and shipping addresses. Not sure how it was solved in Tea Commerce (maybe a custom via a custom order property), but often we need to specific a different email address and telefonenumber for shipping address to be able to send notification from shipping carrier to this person via email and SMS. Would it make sense to add email address property on both billing and shipping in Vendr since we already have this for telefonenumber?

Maybe it would also be useful to move this form to a partial view to be able to use OrderAddressDto model and e.g. @Html.TextBoxFor(x => x.Billing.FirstName) etc.

AddressLine1 and AddressLine2 could maybe be simplified to Line1 and Line2` now since the object is an order address.

[mattbrailsford]

Hi @bjarnef this looks really good. The only thing I would ask is can you re target the PR to be against the dev branch rather than the master one? then I'd be happy to merge it in.

RE the email addresses, I know what you mean, and yes in some cases it would make sense to capture those, and Vendr doesn't stop you from doing that as you can store as much info as you like against the order via order properties. I don't think we necessarily need it in this demo as I our aim really is to keep it as a minimal implementation.

RE the model partials, I'd probably say to not go that route as it just fragments things a little and I like that everything is all together. Again, if implementors want to do that, then they definitely can.

[mattbrailsford]

RE Line1 and Line2 I might be tempted to keep them as they are as yes the object is an address, but when you reference them in code it's not as clear, ie order.Billing.Line1, unless you want to change the Billing property to BillingAddress and then it would make sense order.BillingAddrress.Line1

[bjarnef]

Yes, I think BillingAddress and ShippingAddress might be bette and then have properties Line1 and Line2? Should I change this? :)

An Email or EmailAddress on billing and shipping might also be useful, but not sure how it affect the current setup, where it seems the Email exists on a Customer? Maybe the email address on billing can be the one from customer if nothing else is specified? In UCommerce they have a EmailAddress on Customer, BillingAddress and ShippingAddress (also in database) and where BillingAddress and ShippingAddress are stored as OrderAddress entries with a Name like Billing and Shipment to identify these. https://bitbucket.org/Ucommerce/ucommerce-razor-store/src/d395c7fd0b224a46a9bf7a2b9283f3b5bc3b7c3e/src/uCommerce.RazorStore/Controllers/BillingController.cs#lines-38

[mattbrailsford]

I'm happy to go with the BillingAddress property so yes please.

I'd say leave the Email addresses off the different addresses for now as I'd just prefer to have a slimmer checkout experience for the demo. As I say, anyone who wants them can store them in the order properties themselves.

[bjarnef]

I have updated naming of the properties to BillingAddress and ShippingAddress + Line1 and Line2.

Okay, let's see how it goes. If we need a email address (or company name) for both billing and shipping we have to add these as custom properties for now. How would custom properties be shown and would it be possible to edit these from backoffice? https://github.com/vendrhub/vendr/issues/9

In Tea Commerce:

In Vendr:

Did Tea Commerce had a specific property for "Company"?

[bjarnef]

Okay, in Vendr edit custom details shows "Company" under "Billing", but not under "Shipping".

Just not used in demo store. Not sure what the "Tax Code" under "Billing" is?

[mattbrailsford]

@bjarnef these are controlled via the Order Editor Config file. The default config misses these fields out because not everyone will need them, but they are in the editor view file, so you can "turn them on" by adding them to the order editor config file, for example:

shipping: {
    sameAsBilling: { alias: "shippingSameAsBilling", label: "Same as billing", trueValue: "1", falseValue: "0" },
    firstName: { alias: "shippingFirstName", label: "First Name" },
    lastName: { alias: "shippingLastName", label: "Last Name" },
    company: { alias: "shippingCompany", label: "Company" },
    addressLine1: { alias: "shippingAddressLine1", label: "Street Address Line 1" },
    addressLine2: { alias: "shippingAddressLine2", label: "Street Address Line 2" },
    city: { alias: "shippingCity", label: "City" },
    zipCode: { alias: "shippingZipCode", label: "Zip Code" },
    // Country and Region are already known
    email: { alias: "shippingEmail", label: "Email" },
}

These of course will be documented but I just haven't gotten to that yet.

[mattbrailsford]

@bjarnef Tax Code is like VAT Registration Number.

I'm concerned this PR is getting into a bit of a discussion now and going off on various tangents. If you need to discuss ways to do things or if things are possible, can we move this to the forums and just keep this PR focused on the task at hand, adding in the OrderAddressDto. I'm happy with the changes you've made, but I still need you to re-target this PR to be against the dev branch, not the master.

[bjarnef]

@mattbrailsford just a few things I noticed when working on this, but sorry for moving the focus off topic.

I target the PR to master because it was default branch, but I can re-target it to dev branch.

[mattbrailsford]

It’s cool, I appreciate you spending the time trying stuff out, I’m just making sure things stay on track 👍🏻