Comments

recidive’s picture

FileSize
4.91 KB

An updated patch, making some more fields required and decreasing size of number field.

rszrama’s picture

I can commit this as is, but I don't really know what it takes to have a full address in Brazil. : D

Just so we have the same understanding on what feels to include / require - are only fields that are actually required to mail something required right now? I'm not use to seeing you have to enter so much, including "neighborhood."

Patch seems to work fine as is, though, and I can commit any time.

rszrama’s picture

Actually, I'm about to do some work to add the name fields in, and I don't wanna royally hose your dev repository. I'm going to commit as is and do my work, but please review based on my last comment. I'm guessing you'll need a follow-up patch to accommodate name information anyways.

recidive’s picture

Status: Needs review » Needs work

There might have better xNAL properties for putting Brazilian address info. I'll check on this when I get the chance.

Moving to needs work.

sebas5384’s picture

Status: Needs work » Reviewed & tested by the community

Hi! I live in Brazil,
and it really needs to have some other fields

I already review the patch and according with this document http://xml.coverpages.org/XNAL-DTDs-display.html the fields are right!
So i can't see why this isn't commited, we really need this stuff :D

Cheers!

Damien Tournoud’s picture

Status: Reviewed & tested by the community » Needs work

This patch needs to be rerolled.

rfsbsb’s picture

Status: Needs work » Needs review
FileSize
3.93 KB

Hi,

Here's a new patch taken today.

Please review.

wundo’s picture

Rerolling using previous if clause instead of creating a new if clause for the brazilian address .

barraponto’s picture

Here's a reroll from the proper folder. Reverted changes to "Sao Paulo", since it is the proper transliteration. It is wrapped in t() and can be translated to add proper diacritics (to all states).

pedrorocha’s picture

Is "Neighborhood" really needed? I'm from Brazil too and never sought a website asking me for it.

recidive’s picture

@pedrorocha, neighborhood is usually asked, and it's important, since there are cities with duplicated street names. It should be translated into "Bairro" in pt-br.

barraponto’s picture

Although "bairro" is a usual information when you think of Brazilian addresses, I seldom see them in web forms. Damn colonialism. Supporting is a good sign, and it serves the purpose of disambiguating duplicated street names.

pedrorocha’s picture

Status: Needs review » Reviewed & tested by the community

Sorry, i understood as "referência", but after that i realized it was "Bairro". And obviously, i agree that it should be in every form.

I had a second thought on this case and i think that we could create a specialized module for a brazilian addressfield, because that's why Addressfield module uses CTools plugin system: we can extend it without the need to patch the main module. But i agree that this patch here is a good default and should be commited, without further problems(i'm using it right now).

pedrorocha’s picture

Priority: Normal » Major

Any news?

barraponto’s picture

I'd say breaking Brazilian Address stuff to a new module would be interesting, yet it's a matter of a follow up patch.
Let's commit this.

giorgio79’s picture

barraponto’s picture

I've been wondering whether 'dependent_locality' (a.k.a. Neighborhood) should go into the street_block or locality_block.

It doesn't look right when in locality_block, but I'm not really sure. It does affect the layout of both form and output.

rszrama’s picture

Component: Code » Address formats
Category: task » feature
halth’s picture

Any news on commiting this!? It's very important for us Brazilians to have these fields on the form :)

Mac_Weber’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
10.68 KB

Fixed some stuff on the previous patch and re-rolled it against latest -dev.

@barraponto, the street number must go together with the street address on the xNAL standard (according to rszrama in another issue). Yet, I also don't like storing them on the same field. In addition, premise is not supposed for this kind of value, even on google's schema.

@barraponto, regarding the order of the fields, I followed the Brazilian post office standard guides

Other Brazilians who used the module complained about the labels Address 1 and Address 2. It is fixed, using dependent_locality for "Neighborhood", or bairro in Portuguese. Be aware that google most times return this value as the type sublocality, yet they also have a type called neighborhood.

Other minor fixes:

  • Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
  • Use "elseif" in place of "else if"
recidive’s picture

Status: Needs review » Needs work

@Mac_Weber, you patch is trying to do too much, please clean this up with just the changes in the scope of the issue, leave the coding standards changes to another issue as it makes the patch hard to read and less likely to get in. Please fix code comments to start with capital letters and end with a full stop, again just for the changes in the scope of the issue. Make sure you just touch the parts related to BR.

Thanks

Mac_Weber’s picture

Status: Needs work » Needs review
FileSize
4.05 KB

re-rolled patch file following recidive's comments.

afeijo’s picture

Status: Needs review » Reviewed & tested by the community

Patch works as expected, good job Weber ;)

Mac_Weber’s picture

I also tested removing the fields Full name and Company, it works fine.

How it looks like with this patch:

Edit:

edit Brazilian address form

View:

view Brazilian address form

recidive’s picture

Patch in #22 looks ok to me. Can we get this in?

rszrama’s picture

Status: Reviewed & tested by the community » Fixed

Thanks a lot, guys! Committed.

afeijo’s picture

YAY !

No, thank you rszrama :D

barraponto’s picture

@mac_weber so that's where we should send you a beer?

Status: Fixed » Closed (fixed)

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

pedrorocha’s picture

Status: Closed (fixed) » Active

Great to have this field there!

Although, i came to another doubt that i think is needed to solve the "brazilian address issue": should "sub_premise" field be available too? In Brazil, is very common to have the "Street" separate from the "Number", and the integration with some payment providers in Drupal Commerce needs to improvise to handle this.

Does anybody agree?

Mac_Weber’s picture

@pedrorocha I think you should open a new issue for that.

As a quick reply: sub_premise is not meant to be used as street_number by any known format.

I don't think addressfield will change it because they are following xNAL format, but their maintainers may answer it better.

Mac_Weber’s picture

Status: Active » Closed (fixed)

Closing as this patch is already merged.
As said on #31, a new issue should be opened.

Mac_Weber’s picture

thiago78’s picture

Just a question: How is this different from the solution given by: https://drupal.org/project/br_address ?