Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zip format for Italy #22

Open
masetto opened this issue Oct 7, 2024 · 9 comments
Open

Zip format for Italy #22

masetto opened this issue Oct 7, 2024 · 9 comments

Comments

@masetto
Copy link

masetto commented Oct 7, 2024

I'd like to add Italy zip format and also other interventions to support Italian telephones and addresses.
But first I would like to understand why:

  1. the PRs have not yet been merged from august (smarty3 and civix update). Why? Is this extension supported?
  2. Why in this line is checked if $country == 'CA'? I think it is a typo
  3. Have you ever thought of sending a cumulative email instead of one email per contact with an incorrect zip code?

Thanks

@nganivet
Copy link
Contributor

nganivet commented Oct 7, 2024

Hi @masetto , here are some answers:

  1. this is done on a volunteering basis, sometimes we just need a little nudge ... will look at these by end of week
  2. yes, this looks like a typo, should probably be replaced with array_key_exists($country, $zip_formats) or similar and do a bit of testing ... patch welcomed!
  3. that would make sense, but how would you trigger the cumulative email? on a schedule? by a certain action/condition (ie. civirule)?

@masetto
Copy link
Author

masetto commented Oct 8, 2024

@nganivet thank you for your answer.

I worked on that: 9f66528
But before to make a PR I would ask your opinion about the following points.

Now I'm thinking to work about validation of phone and postal code: your extension simply notify with a message and do not block if a user write a wrong postal code or phone number. Is it intentional?

About no. 3: I'm thinking to add in extension setting an option to add the invalid postal code (and why not phone number?) contacts in a group specified by the user. What do you think?

@nganivet
Copy link
Contributor

nganivet commented Oct 8, 2024

@masetto Yes, it is intentional that we do not block the user. This validation is done from a hook, we do not know if it was triggered by a front-end form, a back-end user, a cron job or an API call, so we cannot 'block' the action. It is a good idea to implement on-screen validation for the CiviCRM zip-code entry field, but that would need to integrate differently in CiviCRM.

It seems like a great idea to have the option to put contacts in a group, that is a good alternative to sending emails.

@masetto
Copy link
Author

masetto commented Oct 8, 2024

@nganivet I create PR #23

I saw that there is the extension Phone Input Mask and I was thinking of adding an option to validate the phone number and the postal code in forms, like that extension. So we can implement the validateForm hook for these form:

  • 'CRM_Contact_Form_Inline_Phone',
  • 'CRM_Contact_Form_Contact',
  • 'CRM_Campaign_Form_Petition_Signature',
  • 'CRM_Contribute_Form_Contribution_Main',
  • 'CRM_Event_Form_Registration_Register',
  • 'CRM_Profile_Form_Edit',

The advantage would be to use a single extension...
My client asks to me to validate postal code and phone (to send SMS), but I think that "background" normalization is however useful....
What do you think?

@nganivet
Copy link
Contributor

nganivet commented Oct 8, 2024

@masetto I did not know about the Phone Input Mask extension, thanks for pointing it out. I would rather have the option to validate the zip codes be rolled into this other extension than adding it here. This way the Normalize extension stays focused on normalizing/validating data once submitted, and the Phone Input Mask deals with the collection and submission in the front-end.
I will link Japp into this discussion to get his take (https://lab.civicrm.org/extensions/phoneinputmask/-/issues/2).

@jaapjansma
Copy link

The phone input mask is specifically for formatting phone numbers in the backend. So back office users get a message when they a enter a non-formatted phone number. So it does not do any normalization of data. It also has a configuration screen to configure which formats are valid formats.

@masetto
Copy link
Author

masetto commented Oct 9, 2024

Yes, how it works phone input mask was clear.
But in my opinion the normalisation process cannot be separated from the validation because by normalising you can have errors and you have to notify them: in fact this extension sends an email or display a message (both not very optimal solutions for me). I think it would be very convenient to add also a validation process (activated by an option) because it already does this using "pre" hook. It would only to implement the validate hook by calling the same normalisation functions, only in this case they would return a message to invalidate the fields.

In any case, I would also need the validation of the postal code and I would not want to create an extension only for that.

@nganivet
Copy link
Contributor

@masetto Not quite sure about what you are suggesting, but you are welcome to do a PR.

@jaapjansma Sorry I misunderstood. I thought your extension provided an input mask on the HTML form widget, similar to when you enter dates or credit card numbers some widget limit accepted chars and insert additional characters as you type.

I see now that this is not the case, so there is substancial overlap between our extensions. My extension mainly does auto-reformatting based on configured policy (ie. enter jajp jansma and it will save Japp JANSMA in the database), but validation features were also added later on, primarily for zip codes.

Ideally we would need the following functions (for US zip code field as example):

  • (front-end, entry mask) place an entry mask on the postal code widget that would restrict entry to digits only, no more than 5
  • (pre-save hooks, validation) trigger an error if zip is not a string of 4 or 5 digits long
  • (pre-save hooks, normalization) add a leading 0 if the zip code is 4 digits long, so 1234 becomes 01234 in database

Phone numbers will be a bit more complex, but still have the need for the entry mask, validation and normalization features.

These features are vastly different in terms of implementation, so could potentially be delegated in different extensions. @masetto outlines the need for common elements between these features. There certainly is symbiosis between entry mask and validation, but I am not sure there is symbiosis with normalization.

@nganivet
Copy link
Contributor

Actually, there is: normalization should validate the field before even trying to reformat it ... makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants