When the Commerce anonymous registration checkout pane is enabled, the login pane should show a nice message near the guest checkout option.

Guest checkout message.

Our login with email checkout pane does not. To do so, we need to implement the canRegisterAfterCheckout method on the pane.

CommentFileSizeAuthor
#2 login-checkout-pane-description-3081857-2.patch1.01 KBlkacenja
Screen Shot 2019-09-17 at 2.17.36 PM.png13.15 KBlkacenja

Issue fork email_registration-3081857

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lkacenja created an issue. See original summary.

lkacenja’s picture

lkacenja
he/him
commented
StatusFileSize
new login-checkout-pane-description-3081857-2.patch1.01 KB

Here is a patch that checks for either the original Commerce pane or our email registration pane.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs review

Looks good to me, though I don't see anything calling that function. Is this patch the whole change?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Ah, I see now this is overriding a method on the parent.

OK, seems reasonable. Leaving this in needs review for a bit to get any feedback from any other users/contributors.

rhovland’s picture

rhovland
he/him
Primary language English
Location Oregon
commented

Thanks for the patch. This fixes the issue where the guest registration message is missing after updating to 1.0.0 and switching to using the email_registration pane.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Is there a way to add testing steps?

lkacenja’s picture

lkacenja
he/him
commented

@andypost did you want steps to reproduce or automated testing steps? I'm happy to provide the former. I could be wrong, but the latter seems difficult to me. This is a fairly deeply embedded Commerce plugin and would require quite a bit of config to test. However, I agree that some test coverage would be really good, as this plugin is not likely to receive ongoing attention. Thanks.

3ssom’s picture

3ssom commented

I'm trying to test this here and I'm hoping I'm doing the step right!

So I've noticed this show happen when the (anonymous registration checkout pane is enabled) so here are the steps and correct me if I'm wrong:

  1. Going to (Checkout flows) and let's say I'm using the default one then Edit it
  2. Under Login or continue as guest there's an option Registration: Not allowed this should be changed to Registration: Allowed then Save
  3. Applied the patch then -R the same patch .. no difference.

Couldn't see the nice message disappear at all! I also tried to put the Login with email registration or continue as guest which was under the Disabled section instead of the default login one but still the same.

am I missing something here?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue tags: +Needs tests

Let's automate that tests to ensure it works now and in the future.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

@greggles what do you think, the tests might be quite heavy due to the Drupal Commerce dependencies and configuration. Should we merge it as-is or better invest the time for the test?
@Grevil what do you think?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: grevil » Unassigned
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I don't have a strong opinion on it. If someone doing the work is willing to do it then that seems great, but I agree it could be hard to get to testing this.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I bet it's not a hard job to add testing, composer req --dev drupal/commerce should work now instead of test_dependencies (used by DrupalCI)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Priority: Normal » Minor
Status: Needs review » Needs work

As of #13

Grevil made their first commit to this issue’s fork.

Grevil opened merge request !17

grevil’s picture

grevil commented

Alright all done! @Anybody, should we show the message only if the "Email registration guest registration after checkout" pane is enabled, or also when the "Guest registration after checkout" is enabled?

grevil’s picture

grevil commented
Assigned: Unassigned » anybody
Status: Needs work » Needs review
grevil’s picture

grevil commented
Assigned: anybody » grevil
Status: Needs review » Needs work

Checking both panes makes only sense. Creating tests now.

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests

Alright, I refactored some of the existing test helping methods, adjusted Testclass names / folder structure and added the required test! Please review! 😊

EDIT: I also moved the helper methods to the top of the class.

grevil’s picture

grevil commented
Status: Needs review » Needs work

Tests fail.

grevil’s picture

grevil commented
Status: Needs work » Needs review

All green now, please review!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Thanks @Grevil for taking the time! :)

  • Anybody committed 362fe061 on 8.x-1.x authored by Grevil 
    Issue #3081857 by Grevil, lkacenja: Login checkout pane should show...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.