I can't see anywhere in the committed code, where I can set the copying
to be the "default action" - and that would IMHO be the normal approach
(is on almost every site I've shopped at) - where the customer
"unchecks" the "my billing is the same as the shipping address" box - if
he/she wants to give a seperate address.
Not the other way around.
The code looks for: '#default_value' =>
isset($order->data['profile_copy'][$checkout_pane['pane_id']]['status'])
? $order->data['profile_copy'][$checkout_pane['pane_id']]['status'] :
FALSE,
I can't see anywhere in the patch, supplying an option to reverse that
logic - but shouldn't it default to TRUE in the above (that works for me
- so the billing fields isn't shown by default - and the checkbox is
ticked - and the order gets the correct billing AND shipping info).
Comments
Comment #1
designbysoil CreditAttribution: designbysoil commentedI was also looking for a way to do this. Should be the default.
Comment #2
rszrama CreditAttribution: rszrama commentedThanks to klavs for opening this follow-up issue to #1410022: Improve copy address functionality - create a more generic solution. There is no option to do this right now, so that must be the starting point - then we can talk about what the default should be. ; )
Comment #3
klavs CreditAttribution: klavs commentedIMHO the simplest, and most correct option, would be to simply add a checkbox, next to the added option stating: "Allow this profile to use values from another profile, where applicable. "
This checkbox would then just say "default to use the value from the other profile, where applicable" or something.
Comment #4
klavs CreditAttribution: klavs commentedJust noticed, that it's not enough to change FALSE to TRUE - for anonymous users - the field being copied to - is still shown - and isn't hidden, until the button is unchecked and then checked again :(
I'll investigate what javascript actually hides the fields and figure out what's up - AFAIK the js wasn't part of the patch that enabled this feature.. so it must be something generic (hints appreciated ;)
Comment #5
delta CreditAttribution: delta commentedIt's not enough to just pass option default value to TRUE. But the problem isn't coming from javascript.
In commerce/modules/customer/includes/commerce_customer.checkout_pane.inc
I change line 124:
to
like described in this issue description, but if we pass this to default, we need to hide the customer profile field when form is created, the same way it is done when the checkbox is checked (via #ajax) so i add this statement on line 146 :
tested this with lastest commerce release and this is working well.
For the hint, when the checkbox is checked the javascript rebuild the customer profile billing or shipping form (depending on your settings) and this code in the form implementation hide the fields
line 136:
}
Comment #6
klavs CreditAttribution: klavs commentedI finally got some time to try this out.
The change at line 146 worked wonders (except for an extra } at the end :)
I would like to convert this into a patch - but would should the orig be?
I am currently running latest stable + http://drupal.org/node/1410022 + the above two snippets.
Comment #7
rszrama CreditAttribution: rszrama commentedTo make a patch, you'd need to do it against the latest 7.x-1.x-dev. You should be able to follow the instructions at http://drupal.org/node/707484 to make this using Git.
Comment #8
klavs CreditAttribution: klavs commentedHere's a patch of the two changes.
This just defaults to enabling copying of pane information (if the admin has selected that this should be a possibility) - and also makes it work for anonymous users.
It doesn't have an option, for the admin to choose default state (enabled/disabled) - but logic (mine atleast) tells me, that if you selected that you want data from 1 field, to be copied from another - you'd want this to be the default - so IMHO it's a fair change.
Comment #9
klavs CreditAttribution: klavs commentedComment #10
delta CreditAttribution: delta commentedThanks to update this as a patch, i'm agree if your check for enabling copy address feature this would be the default. i think..
hope get this is in futur release.
i'm testing it with latest dev release and this is working like a charm
Comment #11
vasikei reworked the #8 patch and added an option for the checkout pane settings for the default copy option.
reviews needed
Comment #13
vasikenew one. it seems the previous wasn't "so good".
Comment #15
rszrama CreditAttribution: rszrama commentedNo worries; the testbot appears to be acting up. I'll ask rfay about it.
Comment #16
rszrama CreditAttribution: rszrama commentedI agree with vasike that this should be optional; no sense not to bake it into the checkout pane settings like this. Let's go ahead and update the test for address copying, though, to test for this behavior for copying being the default vs. not the default.
Comment #17
checker CreditAttribution: checker commentedThank you vasike. Patch #13 looks good. I can't find problems after applying.
Comment #18
checker CreditAttribution: checker commentedstatus should be needs review?
Comment #19
rszrama CreditAttribution: rszrama commentedI had left it in needs work until we got the reverse tests in that would ensure it worked well with address copying as the default.
Comment #20
checker CreditAttribution: checker commentedAfter some testing i guess there a still problems with this patch. It is hard to debug.
- It could be possible that the checkbox is selected by default but the address fieldset is still open.
- Sometimes there are errors about empty address fields although the copy address checkbox is selected.
I am making an journey for the next week maybe someone else can have an eye on this.
Comment #21
bojanz CreditAttribution: bojanz commentedTagging.
Comment #22
andyg5000The existing patch hides all fields on the destination profile even if they don't exist on the source profile.
Here's an updated patch that does the following:
Settings:
Adds a fieldset to the options pane that contains profile copy options when profile copying is enabled (Fieldset is hidden when disabled).
Moves the source profile select box into the fieldset.
Creates a checkbox (TRUE by default) in fieldset for setting profile copy as the default action in checkout.
Checkout
Sets the default value of the copy profile checkbox on checkout to the default action if not set elsewhere.
If profile copy action is set as the default and has not been altered by the customer, it hides all fields that are the same between the source and destination profile.
Tests
I've also updated the test to set the default copy action to TRUE and removed the ajax call that previously copied the profiles.
Comment #23
Carlitus CreditAttribution: Carlitus commented#22 works for me except that the checkbox (Copy profile by default. ) has to be disabled to be enabled in checkout pane by default.
Comment #24
rszrama CreditAttribution: rszrama commentedDropping back to needs work while I give this a little time. I actually meant for us to add a test to check the alternate configuration, not simply change the test that was in there. Wanna make sure we have coverage for both. : )
Comment #25
Carlitus CreditAttribution: Carlitus commented#23 It now works as expected...i don't know why...
Comment #26
Lukas von BlarerWorks for me.
Comment #27
checker CreditAttribution: checker commentedTested #22 with 7.x-1.3+40-dev (2012-Sep-13) without problems.
Comment #28
cvangysel CreditAttribution: cvangysel commentedRe-rolled and reviewed patch against HEAD of 7.x-1.x, including tests for both cases.
Comment #29
rszrama CreditAttribution: rszrama commentedLetting test bot have another stab at this with some changed labels; nothing should've changed substantively since cvangysel's last test, but I couldn't get the tests to work at all locally.
Comment #30
rszrama CreditAttribution: rszrama commentedAlrighty, not sure what was going on locally. Committed.