This a quick "search and replace" duplicate of the Canadian module to provide support for Mexico.
Any advice on how to polish it is welcome ;-)

CommentFileSizeAuthor
#9 cck_address.module_0.patch456 bytesricflomag
#2 cck_address_mexico.tar_.gz2.69 KBricflomag
cck_adress_mexico.tar_.gz2.73 KBricflomag

Comments

rconstantine’s picture

rconstantine commented
Status: Needs review » Needs work

I've done a quick review of your code and mostly everything seems okay.

A couple of questions for you:

1) Are there really two languages that need checking? You have es_MX and es_ES. What are the differences? Can you not get away with one? Or is it only proper to check both?

2) Is that the same ZIP code validation as for Canada? If so, it will probably be wrong. Search the web for a Mexican postal code regular expression. Also, if any other fields will have a particular need, you may need a special regular expression for those as well.

3) I updated the cck_address_canada.module file today. Basically, I just added support for periods in several fields. So streets can be abbreviated St., or Saint can be abbreviated St. and so on. Update your patch to use the updated version.

So once that's all fixed, I'll review it again if you'd like. Do you have a CVS account on Drupal? This would be a good module to get your feet wet with if you don't, since it's nice and small. Although I'll consider putting this up myself, I think that since I know nothing about Mexico, it would be good if it were listed as a separate module and for you to do it. I'm sure that if there were one of these modules for every country, it would be more than I could handle alone. So the sooner I can get help, the better.

If CVS is scary, let me know. Maybe we can work something else out.

Cheers.

ricflomag’s picture

ricflomag commented
StatusFileSize
new cck_address_mexico.tar_.gz2.69 KB

1) I removed es_ES

2) zip codes for Mexico are like in the USA: five digits. So i changed the regex :

    if (($item['zip'] != '') && (!preg_match("/^[0-9]{5}$/", $item['zip']))) {

Is it ok ?

3) I updated the other regex from the new version of the Canada module.

But: i feel that we are duplicating code, which is badTM. I think the cck_address module should add other countries support by loading pure data (and regex). Defining new regex would be optional, and a country that does not provide its own regex for a given field would fall back to the default. Of course, states / provinces / departments data would be compulsory.

Well, this is only a suggestion !

I'd like to open a CVS account on Drupal if you tell me how to ;-)

Regards.

rconstantine’s picture

rconstantine commented

I understand that there appears to be a bit of code reuse, but if you consider Asian, Indian, or other 'exotic' languages/countries and their way of doing addresses, not to mention the validation of that, it makes sense to separate things out. Additionally, I don't want to roll together countries that people don't need. As for myself, I only need the U.S. and will only activate that module. The .info files are only used on the module page. The .install file is only used at install and uninstall. And the .module file only contains a help function, an install helper function, and a validation function. I don't think you're going to find a smaller add-on module scheme out there for any other module.

That said, I do appreciate your thoughts. Don't let this come off as me not wanting to hear what you have to say.

As for CVS, see http://drupal.org/handbook/cvs for more details than you wanted. Particularly, you'll want this: http://drupal.org/node/59

So far as the module goes, it looks fine. Let me know whether you get approved for CVS access. If you have ideas for other modules, include them in your application. It takes about a week for them to review your app and get back to you.

If they don't approve you, I'll go ahead and add your module myself.

Good job.

ricflomag’s picture

ricflomag commented

You're probably right about giving the priority to flexibility over code factorization.

I have uploaded the latest dev release on my site: when installing the canadian (or mexican) country module, php throws the following error:

Fatal error: Call to undefined function cck_address_canada_installstates() in /var/www/vhosts/smo.org.mx/subdomains/asociadas/httpdocs/sites/default/modules/cck_address/countries/cck_address_canada.install on line 13

As i don't have any idea on how Drupal module install works, i haven't tried to guess what's wrong. It must be easy to find out though (the cck_address_canada.module file in not included before the .install is run ?)

I'll have a look at the CVS thing :-)

ricflomag’s picture

ricflomag commented

Doh ! I have pasted the whole system path ! Anyway, i'm about to move the site to another server !

rconstantine’s picture

rconstantine commented

That's strange. The 'include_once' which immediately precedes that function call is supposed to take care of that. By including cck_address_canada.module, the script has access to that function. Try changing 'include_once' to 'require_once'. That may give you a different error message if it is unable to include/require the .module file. Otherwise, it should work. Nothing has changed in that file for quite a while.

And just to make sure, check your .module file for the function that your error message names. I just checked CVS and it looks fine to me.

Let me know what more you find.

ricflomag’s picture

ricflomag commented

The problem is that on some systems (like mine), relative paths are not accepted. The solutions is to do like this (see http://drupal.org/node/117593)

require_once(drupal_get_path('module', 'cck_address') .'/countries/cck_address_canada.module');

Cheers.

rconstantine’s picture

rconstantine commented
Assigned: Unassigned » rconstantine

Thanks. I'll get to this as soon as I can.

ricflomag’s picture

ricflomag commented
StatusFileSize
new cck_address.module_0.patch456 bytes

Here's a patch on the cck_address_get_all_locales() fonction. The ob_start() and ob_end_clean() it used conflicted with Drupal's output and prevented me from making changes on the cck_address_mexico_validate_address_fields() function (i did not understand the whole thing, but the patch i suggest makes my life easier).

rconstantine’s picture

rconstantine commented
Title: Address Field Mexican Support » Address Field Mexican Support & Locale Patch

Did you ever try to get CVS access???

ricflomag’s picture

ricflomag commented

Yes, i've just be given access to CVS (i've had to insist a bit ;-). I'm in a rush at work now, but i think i'll upload my module soon.

rconstantine’s picture

rconstantine commented

Cool. If you use TortoiseCVS, I can help you if you get stuck.

inforeto’s picture

inforeto commented

subscribing

ricflomag’s picture

ricflomag commented
Status: Needs work » Reviewed & tested by the community

It seems that i can't write to the repository:

$ cd ~/drupalCVS/contributions/modules
$ cvs -d :pserver:ricflomag:xxxxxxx@cvs.drupal.org:/cvs/drupal-contrib login
Logging in to :pserver:ricflomag@cvs.drupal.org:2401/cvs/drupal-contrib
$ cvs add cck_address/countries/cck_address_mexico*
cvs [server aborted]: "add" requires write access to the repository

Do i need to be granted write access ?

rconstantine’s picture

rconstantine commented

Interesting. Anybody can access CVS without an account so long as they are doing read-only stuff. But by going through the process of contacting the CVS admins and requesting access is supposed to get you write access. As you said above, that's what you did. I would suggest that you double check all of the CVS documents on Drupal.org in the handbooks section and if you still can't get things going, to contact them.

ricflomag’s picture

ricflomag commented

Looks like i need you to grant me write access to the cck_address/countries directory... Can you do this ?

Hi, that's correct.

The cck_address project is not yours and the current maintainer has not given you write access to their project.

Having a CVS account is only the first step. Best you read those links in the CVS Approve email you got as it's all explained in teh handbook.

regards,
--Andy

rconstantine’s picture

rconstantine commented

Oh, I see. I'll see if I can open a single directory or not. If not, I'll create a new project called cck_address_countries or something and allow folks like you to add and maintain modules there.

I'll post here again when I do that.

rconstantine’s picture

rconstantine commented

Sorry I've been swamped lately. I just finished my latest module, so I'll have time to work on updating my other ones.

I am unable to open subfolders in a project, so what I'm going to do is start a new project using your module to start it off. I will call it cck_address_add_ons or something like that. Inside the project will be folders for the various submissions. So for starters, there will be a mexico folder containing your module. I will then give you CVS access to that project. I'm downloading your module now and should be able to do this tomorrow or the next day.

Thanks for your patience.

rconstantine’s picture

rconstantine commented
Status: Reviewed & tested by the community » Fixed

You now have CVS access here: http://drupal.org/project/cck_address_extensions

ricflomag’s picture

ricflomag commented

I've cleaned the code and loosened the field validation or the Mexican support, and just committed it. I can see my changes in the cvs tree (http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/cck_address...) but i suppose that it is up to you to commit the whole updated cck_address_extensions module. Am i right?

I've left the "Information added by drupal.org packaging script" section of the .info file, i don't know if this will cause trouble.

Please tell me if there is anything i can do to better the process of committing my files. Thank you for your work and patience!

rconstantine’s picture

rconstantine commented

I may not have to do anything that you can't. If on the home page of the project you see a link for "Add new release", then you can add a dev release if you'd like. If you don't see that link, let me know.

ricflomag’s picture

ricflomag commented

wow, it was that easy. thanks.

rconstantine’s picture

rconstantine commented

Good to hear it. If you need further help, check out both the CVS handbook, and if that doesn't answer questions that come up, I'm happy to help.

Now I'll be interested to see if the user that produced the Belgium module gets involved.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)