Comments

Primsi created an issue. See original summary.

primsi’s picture

Initial patch.

primsi’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: commerce_swiss_post-address_validation-3349936-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

primsi’s picture

Status: Needs work » Needs review
StatusFileSize
new11.8 KB

Addressing coding standard issues.

Status: Needs review » Needs work
primsi’s picture

Addressed test fail and added loging.

luksak’s picture

Primsi thank you for your contribution! I'm currently working on other projects. I'm going to have a look into this and provide feedback soon. I can't provide an ETA tough.

berdir’s picture

No worries, Primsi is working with this on me and I'll review it.

We've developed this before for a client project and are open-sourcing it now, basically.

luksak’s picture

Ok, great! Thank you.

One thing a bit off-topic: Could we extend this to support something similar to https://www.drupal.org/project/address_autocomplete_gmaps ?

berdir’s picture

The API we use does have autocomplete endpoints too, so yes. There is a street endpoint that will return suggestions for a full address.

That's not in scope of what we're doing here, but a follow-up could build on that.

The module does implement that by providing its own widget. I do wonder if that would be a more sensible approach to the magic we try to do here right now, would also give us a place to put options, like an autocomplete yes/no checkbox.

luksak’s picture

@berdir sure, a follow-up makes more sense.

Exactly, that's why I suggested it here. But the autocomplete approach also comes with a lot of disadvantages, if solved poorly and can result in a worse UX than before. address_autocomplete_gmaps is actually a good example. It breaks the address form's UX quite a bit. For example editing the values again is quite cumbersome.

There is a typo in the service's name: `commerce_swiss_post.adress_verification` It should be: `commerce_swiss_post.address_verification`.

primsi’s picture

Implemented the idea from #11 and fixed the naming issue from #12

primsi’s picture

Forgot to mention in the previous comment: also added the ability to skip validation with a double submission of the same address.

Here adding a basic webform validation handler. Not sure if we want to add he auto-correcting functionality here as well.

berdir’s picture

I think we want the same behavior there, auto-correct and also the resubmit part. Otherwise looks good I think.

primsi’s picture

luksak’s picture

One more question; Do we really need sessions for this? Can't the re-submission functionality be achieved with form state? It feels weird to store all addresses in sessions...

primsi’s picture

I did try storing it in form state, but then, this gets passed to later steps, which would need to be handled. With session there is no such issue. But I am open to try again if we think this is better.

primsi’s picture

Fixing and improving stuff from internal testing.

1. Fix log levels not being correctly used
2. Rephrase debug messages.
3. Fix issue where a wrong address can be submitted if limit_validation_errors was used because the old imput was stored

Status: Needs review » Needs work

The last submitted patch, 19: commerce_swiss_post-address_validation-3349936-19.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

primsi’s picture

Status: Needs work » Needs review
StatusFileSize
new22.42 KB

Removing the cron bit unintentionally added to the patch.

primsi’s picture

As per discussion with @Berdir, readdig the cron bit with the correct module name (not entity type name as before).

  • Berdir committed 6843589c on 1.0.x authored by Primsi
    Issue #3349936 by Primsi, Lukas von Blarer, Berdir: Add swiss post...
berdir’s picture

Status: Needs review » Fixed

This was tested and does what it's meant to. There are some possible additional feature and improvements, like configurable action on returned changes (some are a bit strange) as well as supporting autocomplete in some form as well, but we can look into that if someone is interested in sponsoring that.

Status: Fixed » Closed (fixed)

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